Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SPM install + Add secure #422

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

Fix SPM install + Add secure #422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3e68be4
Update Package.swift
matteocrippa Apr 17, 2021
19df321
Update Package.swift
matteocrippa Apr 17, 2021
619f2b7
introduce secure storage
matteocrippa Apr 19, 2021
0d3928c
Update SQLiteStore.swift
matteocrippa Apr 19, 2021
6eac860
Update SQLiteStore.swift
matteocrippa Apr 19, 2021
e7314ee
add infoplist to excluded to silent error on spm
matteocrippa Apr 29, 2021
5370f39
Merge branch 'JohnEstropia:develop' into feature/spm-fix-secure
matteocrippa May 15, 2021
02156db
avoid force cast
matteocrippa May 15, 2021
5ab5fd2
remove force try!
matteocrippa Jun 7, 2021
cf636f2
rollback
matteocrippa Jun 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version:5.0
// swift-tools-version:5.3
//
// Package.swift
// CoreStore
Expand Down Expand Up @@ -40,7 +40,7 @@ let package = Package(
name: "CoreStore",
dependencies: [],
path: "Sources",
exclude: ["CoreStoreBridge.h", "CoreStoreBridge.m"]
exclude: ["CoreStoreBridge.h", "CoreStoreBridge.m", "ObjectPublisher+Reactive.swift", "Info.plist"]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these exclusions needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was generating error on compiling if I didn't exclude that

),
.testTarget(
name: "CoreStoreTests",
Expand Down
3 changes: 2 additions & 1 deletion Sources/From.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ public struct From<O: DynamicObject> {
)
throw CoreStoreError.unknown
}
fetchRequest.entity = parentStack.entityDescription(for: Internals.EntityIdentifier(self.entityClass))!
guard let entity = parentStack.entityDescription(for: Internals.EntityIdentifier(self.entityClass)) else { return }
fetchRequest.entity = entity
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What specific use-case does this resolve?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

guard applyAffectedStores else {

return
Expand Down
9 changes: 7 additions & 2 deletions Sources/SQLiteStore.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,13 @@ public final class SQLiteStore: LocalStorage {

- 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) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be good, I had tried a quick approach without too many changes, prob a subclass is even better


self.fileURL = SQLiteStore.defaultFileURL
self.configuration = nil
self.migrationMappingProviders = []
self.localStorageOptions = nil
self.secure = secure
}

/**
Expand Down Expand Up @@ -133,7 +134,8 @@ public final class SQLiteStore: LocalStorage {


// MARK: StorageInterface

private var secure: Bool = false

/**
The string identifier for the `NSPersistentStore`'s `type` property. For `SQLiteStore`s, this is always set to `NSSQLiteStoreType`.
*/
Expand Down Expand Up @@ -217,6 +219,9 @@ public final class SQLiteStore: LocalStorage {

var storeOptions = self.storeOptions ?? [:]
storeOptions[NSSQLitePragmasOption] = ["journal_mode": "DELETE"]
if secure {
storeOptions[NSPersistentStoreFileProtectionKey] = FileProtectionType.complete
}
try coordinator.addPersistentStore(
ofType: Self.storeType,
configurationName: self.configuration,
Expand Down