Skip to content

Commit 4361f0c

Browse files
bassrockbassrock
authored
fix(saved-item): ensuring saved item only is pulled by url (#665)
fix(saved-item): ensuring saved item only is pulled by urll
1 parent 8451e0b commit 4361f0c

File tree

13 files changed

+30
-56
lines changed

13 files changed

+30
-56
lines changed

‎PocketKit/Sources/PocketKit/MyList/ItemsList/PocketItemViewModel.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,9 @@ class PocketItemViewModel: ObservableObject {
145145
/// Fetch a SavedItem or create one in order to use actions related to source
146146
private func fetchSavedItem() -> SavedItem? {
147147
guard
148-
let id = item.id,
148+
let url = item.url,
149149
let savedItem = source.fetchOrCreateSavedItem(
150-
with: id,
150+
with: url,
151151
and: item.remoteItemParts
152152
)
153153
else {

‎PocketKit/Sources/PocketKit/MyList/SearchItemsList/SearchViewModel.swift

+4-4
Original file line numberDiff line numberDiff line change
@@ -406,9 +406,9 @@ extension SearchViewModel {
406406

407407
func select(_ searchItem: PocketItem, index: Int) {
408408
guard
409-
let id = searchItem.id,
409+
let url = searchItem.url,
410410
let savedItem = source.fetchOrCreateSavedItem(
411-
with: id,
411+
with: url,
412412
and: searchItem.remoteItemParts
413413
)
414414
else {
@@ -472,9 +472,9 @@ extension SearchViewModel {
472472

473473
func fetchSavedItem(_ searchItem: PocketItem) -> SavedItem? {
474474
guard
475-
let id = searchItem.id,
475+
let url = searchItem.url,
476476
let savedItem = source.fetchOrCreateSavedItem(
477-
with: id,
477+
with: url,
478478
and: searchItem.remoteItemParts
479479
)
480480
else {

‎PocketKit/Sources/Sync/PocketSource.swift

+4-4
Original file line numberDiff line numberDiff line change
@@ -880,21 +880,21 @@ extension PocketSource {
880880
try? space.fetchSavedItems(bySearchTerm: search, userPremium: user.status == .premium)
881881
}
882882

883-
public func fetchOrCreateSavedItem(with remoteID: String, and remoteParts: SavedItem.RemoteSavedItem?) -> SavedItem? {
884-
let savedItem = (try? space.fetchSavedItem(byRemoteID: remoteID))
883+
public func fetchOrCreateSavedItem(with url: URL, and remoteParts: SavedItem.RemoteSavedItem?) -> SavedItem? {
884+
let savedItem = (try? space.fetchSavedItem(byURL: url))
885885

886886
if let remoteParts {
887887
savedItem?.update(from: remoteParts, with: space)
888888
}
889889

890-
guard savedItem == nil, let remoteParts, let url = URL(string: remoteParts.url) else {
890+
guard savedItem == nil, let remoteParts else {
891891
Log.breadcrumb(category: "sync", level: .debug, message: "SavedItem found and don't need to create one")
892892
// save the space with the updated item data
893893
try? space.save()
894894
return savedItem
895895
}
896896

897-
let remoteSavedItem = SavedItem(context: space.backgroundContext, url: url, remoteID: remoteID)
897+
let remoteSavedItem = SavedItem(context: space.backgroundContext, url: url, remoteID: remoteParts.remoteID)
898898
remoteSavedItem.update(from: remoteParts, with: space)
899899
try? space.save()
900900

‎PocketKit/Sources/Sync/Requests.swift

-21
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,6 @@ public enum Requests {
3737
return request
3838
}
3939

40-
public static func fetchSavedItem(byRemoteID remoteID: String) -> NSFetchRequest<SavedItem> {
41-
let request = SavedItem.fetchRequest()
42-
request.predicate = NSPredicate(format: "remoteID = %@", remoteID)
43-
request.fetchLimit = 1
44-
return request
45-
}
46-
47-
public static func fetchSavedItem(byRemoteItemID remoteItemID: String) -> NSFetchRequest<SavedItem> {
48-
let request = SavedItem.fetchRequest()
49-
request.predicate = NSPredicate(format: "item.remoteID = %@", remoteItemID)
50-
request.fetchLimit = 1
51-
return request
52-
}
53-
5440
public static func fetchSavedItem(byURL url: URL) -> NSFetchRequest<SavedItem> {
5541
let request = SavedItem.fetchRequest()
5642
request.predicate = NSPredicate(format: "url.absoluteString = %@", url.absoluteString)
@@ -135,13 +121,6 @@ public enum Requests {
135121
Item.fetchRequest()
136122
}
137123

138-
public static func fetchItem(byRemoteID id: String) -> NSFetchRequest<Item> {
139-
let request = self.fetchItems()
140-
request.predicate = NSPredicate(format: "remoteID = %@", id)
141-
request.fetchLimit = 1
142-
return request
143-
}
144-
145124
public static func fetchSyndicatedArticles() -> NSFetchRequest<SyndicatedArticle> {
146125
SyndicatedArticle.fetchRequest()
147126
}

‎PocketKit/Sources/Sync/Source.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public protocol Source {
8484

8585
func searchSaves(search: String) -> [SavedItem]?
8686

87-
func fetchOrCreateSavedItem(with remoteID: String, and remoteParts: SavedItem.RemoteSavedItem?) -> SavedItem?
87+
func fetchOrCreateSavedItem(with url: URL, and remoteParts: SavedItem.RemoteSavedItem?) -> SavedItem?
8888

8989
/// Get the count of unread saves
9090
/// - Returns: Int of unread saves

‎PocketKit/Sources/Sync/Space/DerivedSpace.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ extension DerivedSpace: SavedItemSpace {
8888
logItemUpdated(itemID: node.remoteID)
8989

9090
context.performAndWait {
91-
let item = (try? space.fetchSavedItem(byRemoteID: node.remoteID, context: context)) ?? SavedItem(context: context, url: url, remoteID: node.remoteID)
91+
let item = (try? space.fetchSavedItem(byURL: url, context: context)) ?? SavedItem(context: context, url: url, remoteID: node.remoteID)
9292
item.update(from: edge, with: space)
9393

9494
if item.deletedAt != nil {
@@ -113,7 +113,7 @@ extension DerivedSpace: ArchivedItemSpace {
113113
logItemUpdated(itemID: node.remoteID)
114114

115115
context.performAndWait {
116-
let item = (try? space.fetchSavedItem(byRemoteID: node.remoteID, context: context)) ?? SavedItem(context: context, url: url, remoteID: node.remoteID)
116+
let item = (try? space.fetchSavedItem(byURL: url, context: context)) ?? SavedItem(context: context, url: url, remoteID: node.remoteID)
117117
item.update(from: node.fragments.savedItemSummary, with: space)
118118
item.cursor = edge.cursor
119119
if item.deletedAt != nil {

‎PocketKit/Sources/Sync/Space/Space.swift

+2-6
Original file line numberDiff line numberDiff line change
@@ -208,12 +208,8 @@ extension Space {
208208

209209
// MARK: SavedItem
210210
extension Space {
211-
func fetchSavedItem(byRemoteID remoteID: String, context: NSManagedObjectContext? = nil) throws -> SavedItem? {
212-
return try fetch(Requests.fetchSavedItem(byRemoteID: remoteID), context: context).first
213-
}
214-
215-
func fetchSavedItem(byRemoteItemID remoteItemID: String) throws -> SavedItem? {
216-
return try fetch(Requests.fetchSavedItem(byRemoteItemID: remoteItemID)).first
211+
func fetchSavedItem(byURL url: URL, context: NSManagedObjectContext? = nil) throws -> SavedItem? {
212+
return try fetch(Requests.fetchSavedItem(byURL: url), context: context).first
217213
}
218214

219215
func fetchSavedItem(byURL url: URL) throws -> SavedItem? {

‎PocketKit/Tests/PocketKitTests/Support/MockSource.swift

+5-5
Original file line numberDiff line numberDiff line change
@@ -1209,23 +1209,23 @@ extension MockSource {
12091209
// MARK: - Fetch SavedItem by Remote ID
12101210
extension MockSource {
12111211
private static let fetchSavedItem = "fetchSavedItem"
1212-
typealias FetchSavedItemImpl = (String) -> SavedItem?
1212+
typealias FetchSavedItemImpl = (URL) -> SavedItem?
12131213

12141214
struct FetchSavedItemCall {
1215-
let remoteID: String
1215+
let url: URL
12161216
}
12171217

12181218
func stubFetchSavedItem(impl: @escaping FetchSavedItemImpl) {
12191219
implementations[Self.fetchSavedItem] = impl
12201220
}
12211221

1222-
func fetchOrCreateSavedItem(with remoteID: String, and remoteParts: SavedItem.RemoteSavedItem?) -> SavedItem? {
1222+
func fetchOrCreateSavedItem(with url: URL, and remoteParts: SavedItem.RemoteSavedItem?) -> SavedItem? {
12231223
guard let impl = implementations[Self.fetchSavedItem] as? FetchSavedItemImpl else {
12241224
fatalError("\(Self.self).\(#function) has not been stubbed")
12251225
}
12261226

1227-
calls[Self.fetchSavedItem] = (calls[Self.fetchSavedItem] ?? []) + [FetchSavedItemCall(remoteID: remoteID)]
1228-
return impl(remoteID)
1227+
calls[Self.fetchSavedItem] = (calls[Self.fetchSavedItem] ?? []) + [FetchSavedItemCall(url: url)]
1228+
return impl(url)
12291229
}
12301230

12311231
func fetchSavedItemCall(at index: Int) -> FetchSavedItemCall? {

‎PocketKit/Tests/SyncTests/Fixtures/save-item.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"upsertSavedItem": {
44
"__typename": "[type-name-here]",
55
"remoteID": "saved-item-1",
6-
"url": "https://example.com/item-1",
6+
"url": "http://example.com/add-me-to-your-list",
77
"_createdAt": 0,
88
"_deletedAt": null,
99
"archivedAt": null,

‎PocketKit/Tests/SyncTests/Operations/SaveItemOperationTests.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ class SaveItemOperationTests: XCTestCase {
7070
XCTAssertNotNil(performCall)
7171
XCTAssertEqual(performCall?.mutation.input.url, url.absoluteString)
7272

73-
let item = try space.fetchSavedItem(byRemoteID: "saved-item-1")
73+
let item = try space.fetchSavedItem(byURL: url)
7474
XCTAssertEqual(savedItem.item.resolvedURL, URL(string: "https://resolved.example.com/item-1")!)
7575
XCTAssertNotNil(item)
7676
}

‎PocketKit/Tests/SyncTests/PocketSaveServiceTests.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ class PocketSaveServiceTests: XCTestCase {
109109
return MockCancellable()
110110
}
111111

112-
let url = URL(string: "https://example.com/a-new-item")!
112+
let url = URL(string: "http://example.com/add-me-to-your-list")!
113113
let service = subject()
114114
_ = service.save(url: url)
115115

@@ -122,7 +122,7 @@ class PocketSaveServiceTests: XCTestCase {
122122

123123
do {
124124
wait(for: [performMutationCompleted], timeout: 10)
125-
let savedItem = try space.fetchSavedItem(byRemoteID: "saved-item-1")
125+
let savedItem = try space.fetchSavedItem(byURL: url)
126126
XCTAssertNotNil(savedItem?.item)
127127
}
128128
}

‎PocketKit/Tests/SyncTests/PocketSourceTests.swift

+4-6
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ class PocketSourceTests: XCTestCase {
166166
}
167167

168168
func test_delete_removesItemFromLocalStorage_andExecutesDeleteMutation() throws {
169-
let item = try space.createSavedItem(remoteID: "delete-me")
169+
let item = try space.createSavedItem(remoteID: "delete-me", url: "https://mozilla.com/delete")
170170
let expectationToRunOperation = expectation(description: "Run operation")
171171
operations.stubItemMutationOperation { (_, _, _: DeleteItemMutation) in
172172
TestSyncOperation {
@@ -177,7 +177,7 @@ class PocketSourceTests: XCTestCase {
177177
let source = subject()
178178
source.delete(item: item)
179179

180-
let fetchedItem = try space.fetchSavedItem(byRemoteID: "delete-me")
180+
let fetchedItem = try space.fetchSavedItem(byURL: URL(string: "https://mozilla.com/delete")!)
181181
XCTAssertNil(fetchedItem)
182182
XCTAssertFalse(item.hasChanges)
183183
wait(for: [expectationToRunOperation], timeout: 10)
@@ -237,7 +237,7 @@ class PocketSourceTests: XCTestCase {
237237
}
238238

239239
func test_unarchive_executesSaveItemMutation_andUpdatesCreatedAtField() throws {
240-
let item = try space.createSavedItem(remoteID: "unarchive-me")
240+
let item = try space.createSavedItem(remoteID: "unarchive-me", url: "https://mozilla.com/unarchive")
241241
item.isArchived = true
242242

243243
let expectationToRunOperation = expectation(description: "Run operation")
@@ -250,8 +250,6 @@ class PocketSourceTests: XCTestCase {
250250
let source = subject()
251251
source.unarchive(item: item)
252252

253-
let fetchedItem = try space.fetchSavedItem(byRemoteID: "archive-me")
254-
XCTAssertNil(fetchedItem)
255253
XCTAssertFalse(item.isArchived)
256254
XCTAssertNotNil(item.createdAt)
257255
wait(for: [expectationToRunOperation], timeout: 10)
@@ -699,7 +697,7 @@ extension PocketSourceTests {
699697
)
700698

701699
let source = subject()
702-
let savedItem = source.fetchOrCreateSavedItem(with: "saved-item", and: itemParts)
700+
let savedItem = source.fetchOrCreateSavedItem(with: URL(string: "http://localhost:8080/hello")!, and: itemParts)
703701

704702
XCTAssertEqual(savedItem?.remoteID, "saved-item")
705703
XCTAssertEqual(savedItem?.item.title, "item-title")

‎PocketKit/Tests/SyncTests/Support/FetchSavesTests.swift

+2-1
Original file line numberDiff line numberDiff line change
@@ -149,13 +149,14 @@ class FetchSavesTests: XCTestCase {
149149
apollo.setupFetchSavesSyncResponse(listFixtureName: "updated-item")
150150
try space.createSavedItem(
151151
remoteID: "saved-item-1",
152+
url: "http://example.com/item-1",
152153
item: space.buildItem(title: "Item 1")
153154
)
154155

155156
let service = subject()
156157
_ = await service.execute(syncTaskId: task.objectID)
157158

158-
let item = try space.fetchSavedItem(byRemoteID: "saved-item-1")
159+
let item = try space.fetchSavedItem(byURL: URL(string: "http://example.com/item-1")!)
159160
XCTAssertEqual(item?.item.title, "Updated Item 1")
160161
}
161162

0 commit comments

Comments
 (0)