Skip to content

Commit

Permalink
Fixed issue where component was not being stored and looked up by sam…
Browse files Browse the repository at this point in the history
…e cache key.
  • Loading branch information
vincethecoder committed Jun 25, 2018
1 parent fa1b32c commit bff3b2f
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
5 changes: 3 additions & 2 deletions MapboxNavigation/InstructionPresenter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,13 @@ class InstructionPresenter {
let attachment = ExitAttachment()
guard let cacheKey = component.cacheKey else { return nil }

if let image = imageRepository.cachedImageForKey(cacheKey) {
let key = [cacheKey, additionalKey].joined(separator: "-")
if let image = imageRepository.cachedImageForKey(key) {
attachment.image = image
} else {
let view = ExitView(pointSize: dataSource.font.pointSize, side: side, text: text)
guard let image = takeSnapshot(on: view) else { return nil }
imageRepository.storeImage(image, forKey: [cacheKey, additionalKey].joined(separator: "-"), toDisk: false)
imageRepository.storeImage(image, forKey: key, toDisk: false)
attachment.image = image
}

Expand Down
10 changes: 10 additions & 0 deletions MapboxNavigationTests/InstructionsBannerViewIntegrationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,9 @@ class InstructionsBannerViewIntegrationTests: XCTestCase {
let presenter = InstructionPresenter(exitInstruction, dataSource: label, downloadCompletion: nil)
let attributed = presenter.attributedText()

let key = cacheKeyForExitLabel(dataSource: label, cacheKey: exitCodeAttribute.cacheKey!)!
XCTAssertNotNil(imageRepository.cachedImageForKey(key), "Expected cached image")

let spaceRange = NSMakeRange(1, 1)
let space = attributed.attributedSubstring(from: spaceRange)
//Do we have spacing between the attachment and the road name?
Expand All @@ -282,6 +285,13 @@ class InstructionsBannerViewIntegrationTests: XCTestCase {
let roadName = attributed.attributedSubstring(from: roadNameRange)
XCTAssert(roadName.string == "Main Street", "Banner not populating road name correctly")
}

private func cacheKeyForExitLabel(side: ExitSide = .right, dataSource: InstructionPresenter.DataSource, cacheKey: String) -> String? {

This comment has been minimized.

Copy link
@akitchen

akitchen Jun 25, 2018

Contributor

Hmm, copy & pasting this between the implementation and test might indicate that there's an abstraction missing. I'd rather our tests not duplicate this effort from the implementation; could you please revisit?

let proxy = ExitView.appearance()
let criticalProperties: [AnyHashable?] = [side, dataSource.font.pointSize, proxy.backgroundColor, proxy.foregroundColor, proxy.borderWidth, proxy.cornerRadius]
let additionalKey = String(describing: criticalProperties.reduce(0, { $0 ^ ($1?.hashValue ?? 0)}))
return [cacheKey, additionalKey].joined(separator: "-")
}

private func simulateDownloadingShieldForComponent(_ component: VisualInstructionComponent) {
let operation: ImageDownloadOperationSpy = ImageDownloadOperationSpy.operationForURL(component.imageURL!)!
Expand Down

0 comments on commit bff3b2f

Please sign in to comment.