Fix SPM install + Add secure#422
Conversation
JohnEstropia
left a comment
JohnEstropia
left a comment
There was a problem hiding this comment.
Hi, thanks for this PR and for raising the issues presented here. I left some questions and I'll make considerations based on your answers. As for the file protection support, I'll have to think of a tighter solution for it since once those flags are on, a lot of disk behaviors start to affect the save processing. In the meantime, I'll keep this PR open for now.
| } | ||
| fetchRequest.entity = parentStack.entityDescription(for: Internals.EntityIdentifier(self.entityClass))! | ||
| guard let entity = parentStack.entityDescription(for: Internals.EntityIdentifier(self.entityClass)) else { return } | ||
| fetchRequest.entity = entity |
There was a problem hiding this comment.
What specific use-case does this resolve?
There was a problem hiding this comment.
I was facing an error when debugging, dunno if related to change of the store in a secure way, so I preferred to guard and avoid a strong crash
| dependencies: [], | ||
| path: "Sources", | ||
| exclude: ["CoreStoreBridge.h", "CoreStoreBridge.m"] | ||
| exclude: ["CoreStoreBridge.h", "CoreStoreBridge.m", "ObjectPublisher+Reactive.swift", "Info.plist"] |
There was a problem hiding this comment.
Why are these exclusions needed?
There was a problem hiding this comment.
Was generating error on compiling if I didn't exclude that
| - Warning: The default SQLite file location for the `LegacySQLiteStore` and `SQLiteStore` are different. If the app was depending on CoreStore's default directories prior to 2.0.0, make sure to use the `SQLiteStore.legacy(...)` factory methods to create the `SQLiteStore` instead of using initializers directly. | ||
| */ | ||
| public init() { | ||
| public init(secure: Bool = false) { |
There was a problem hiding this comment.
I'd rather implement this as a separate store, or a subclass of SQLiteStore. The use of NSFileProtectionTypes have some nuanced effects, and the errors that may occur need to be expressed in CoreStoreError.
There was a problem hiding this comment.
Can be good, I had tried a quick approach without too many changes, prob a subclass is even better
2 participants
This PR aims to fix one issue for SPM install:
Also it introduces and extra init param that will force the sqlite file to be encrypted ( I took as reference #145 )