Conversation
|
@pgawlowski: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
designatednerd
left a comment
There was a problem hiding this comment.
@pgawlowski I like the change to enable use of SQLCipher, but I think adding it as a dependency is a mistake.
| ss.dependency 'Apollo/Core' | ||
| ss.dependency 'SQLCipher', '~>4.2.0' | ||
| ss.dependency 'SQLite.swift/SQLCipher', '> 0.12.0' | ||
| end |
There was a problem hiding this comment.
We're not actually using anything from SQLite cipher inside our library, so I don't think this is necessary (and would add a couple of dependencies that could cause conflicts.
There was a problem hiding this comment.
The main problem is - it seems to be impossible to use it together with Apollo/SQLite.
SQLite.swift/SQLCipher - requires only this particular SQLite dependency.
Mixing it together withss.dependency 'SQLite.swift', '~>0.12.2' results in not working SQLCipher layer. It is not going to cause any error, crash or compilation warning. Just DB is not going to encrypt at all.
That is why I prepared separate dependency.
Please check this comment and the answer:
stephencelis/SQLite.swift#570 (comment)
This is the exact situation.
I was already trying to use
pod Apollo/SQLite
'pod 'SQLCipher', '~>4.2.0'
'pod 'SQLite.swift/SQLCipher', '> 0.12.0'
together with this modified init
self.shouldVacuumOnClear = shouldVacuumOnClear
self.db = db
try self.createTableIfNeeded()
}
and DB is not encrypting.
@edit:
To avoid this dependency we would have to introduce SQLitenormalizedCache ... without SQLite in apollo dependencies to provide it from the outside with
pod 'SQLCipher', '~>4.2.0'
pod 'SQLite.swift/SQLCipher', '> 0.12.0'
Any thoughts how to resolve?
There was a problem hiding this comment.
Oof, looking at the SQLitePodspec it looks like there's some additional xcconfigs that get set up in order to make that work. Looking at the README for FMDB, another iOS SQLite lib, I see a similar caveat about how to set up the podspec.
Looking at the tutorial on SQLCipher, there's a fairly blunt caveat about only using their SQLite 3:
Neither SQLite.swift nor FMDB support using this through anything other than CocoaPods, which is a big old red flag for me in terms of maintaining support on the Apollo side, which would be necessary if we were to add this to the library.
Honestly, at this point, it feels like we're pretzeling ourselves to make this work in a way that just doesn't make sense for the overwhelming majority of users. This is a major reason why NormalizedCache is a protocol: so that you can implement your own version of it if the one we provide is insufficient.
| self.shouldVacuumOnClear = shouldVacuumOnClear | ||
| self.db = db | ||
| try self.createTableIfNeeded() | ||
| } |
There was a problem hiding this comment.
This seems like a reasonable addition in terms of allowing people to use SQLCipher or other extensions to SQLite.swift, though, so I do think we should keep this. Can you add some documentation to this initializer please?
|
I'm gonna close this one out - I went into detail in this comment about some of my concerns but ultimately, this isn't something we have the bandwidth to be supporting in the main library due to the complexity of managing this dependency. Probably the easiest way to make this work would be to:
It's not a great answer, but it looks like it's not any easier with FMDB or even just using SQLCipher directly. |
|
@designatednerd Hi, given the outcome of this I'm wondering if we could still put in the change outlined here to pass in the encrypted DB Connection object (#1162 (comment)) |
|
Yes if you'd like to open a separate PR with that change, that'd be lovely. |

Hello there :)
Based on this discussion #1117
I thought that I will give it a shot and try to introduce SQLCipher integration.
Usage is quite simple.
pod 'Apollo/SQLiteCipher'Apollo.podspec modification is a result of this issue:
stephencelis/SQLite.swift#570
It is impossible to integrate those two:
pod 'Apollo/SQLite'pod 'SQLite.swift/SQLCipher'Cause the result is normal SQLiteDB which is unable to use Cipher encryption. Probably it is conflicting somehow with normal SQLite implementation.
I did not introduce SPM packged denepdency - not sure how to handle this part to be honest.