SwiftDataReplaceTable.swift

func replaceCachedContacts(_ contacts: [UserOrContact]) throws {
        let modelContext = ModelContext(modelContainer)

        let internalIdSet = Set(contacts.map { $0.internalId })

        var internalIdsToNotInsert = Set<String>()
        var existingInternalIdsToDelete = Set<String>()

        let existingContacts = try modelContext.fetch(
            FetchDescriptor<UserOrContact>(
                predicate: #Predicate { internalIdSet.contains($0.internalId) }
            )
        )

        let internalIdsToExistingContacts = existingContacts
            .reduce(into: [String: UserOrContact]()) {
                $0[$1.internalId] = $1
            }

        for contact in contacts {
            if contact.equalProperties(to: internalIdsToExistingContacts[contact.internalId]) {
                internalIdsToNotInsert.insert(contact.internalId)
            } else {
                existingInternalIdsToDelete.insert(contact.internalId)
            }
        }

        try modelContext.transaction {
            // Delete models which have changed or – if deleteOthers is true – do not exist in
            // the models parameter
            try modelContext.delete(model: UserOrContact.self, where: #Predicate {
                existingInternalIdsToDelete.contains($0.internalId) || !internalIdSet.contains($0.internalId)
            })

            // Insert new and changed models
            for contact in contacts {
                guard !internalIdsToNotInsert.contains(contact.internalId) else {
                    continue
                }

                modelContext.insert(contact)
            }
        }
    }

Yeah, I was going to post that that upsert function in your GitHub repo wasn't going to work once you set up your own identifier.

A few observations:

1. If you haven’t already, I might advise defining the internalId to be unique. That serves two purposes:

   • It makes sure you don't accidentally have duplicate internalIds in your table.
   • It appears to enforce this uniqueness by adding a unique index on that column, which will likely improve the performance of queries to check to see if the item already exists.

   E.g.:

   @Model
   final class UserOrContact {
       @Attribute(.unique) var internalId: String
       …
   
       init(_ itemId: String) {
           self.itemId = itemId
   
           …
       }
   }

2. This may be a stylistic observation, but I question the idea of creating a whole new set of UserOrContact objects that you pass to this function, many of which you may not ever persist because the value found in SwiftData is unchanged. Because that is a @Model object (presumably), that means that it is creating a brand new PersistentModel instance (with all the internals that entails), but if the object is deemed to be unchanged, you are just discarding it. It probably isn’t going to introduce problems, but seems conceptually incorrect. It just feels wrong to create persistable objects that are never persisted.

   It seems like the appropriate pattern would be to pass an array of non-PersistentModel objects and that you would iterate through those and find the existing UserOrContact. If found, merely update its properties. If not, insert. And at the end, remove any old objects in the context for which the internalId lookup didn’t get a hit.

   I might also benchmark a far simpler pattern, namely delete all and reinsert all. If the performance is indistinguishable, sometimes simple is better than smart.

3. Minor observation, but …

   for contact in contacts {
       guard !internalIdsToNotInsert.contains(contact. internalId) else {
           continue
       }
   
       modelContext.insert(contact)
   }

   … can be simplified to:

   for contact in contacts where !internalIdsToNotInsert.contains(contact. internalId) {
       modelContext.insert(contact)
   }

   That is just syntactic sugar, but makes it easier to read, IMHO.

4. Likewise…

   let internalIdsToExistingContacts = existingContacts
       .reduce(into: [String: UserOrContact]()) {
           $0[$1.internalId] = $1
       }

   … could conceivably be replaced with:

   let internalIdsToExistingContacts: [String: Item] = existingContacts
       .reduce(into: [:]) {
           $0[$1.internalId] = $1
       }

   Or:

   let foo = Dictionary(existingContacts.map { ($0.internalId, $0) }) {
       _, second in second
   }

   But, that is definitely is just a matter of personal taste, so feel free to disregard if you’d like. Lol.

Hey Robert! First off, really appreciate you taking the time to provide some feedback. It's all very helpful and I've incorporated all of it into my project.

It's an excellent point to avoid instantiating PersistentModel instances for objects that will (very likely) be discarded; using a struct instead should improve the performance here.