mergesort / bodega Goto Github PK

    public func readDataAndKeys(keys: [CacheKey]) async -> [(key: CacheKey, data: Data)] {
        return zip(
            keys,
            await self.read(keys: keys)
        ).map { ($0, $1) }
    }

self.read(keys: keys) returns an array of data, omitting values to nonexistent keys. However, there's no way for this method to know which keys were omitted.

This means that if an array of keys is passed in with a nonexistent key in the first position, all the subsequent keys will be associated with the wrong Data.

I noticed that even after calling removeAllData, the size of the sqlite3 file doesn't shrink. This is because sqlite reserves the freed space for future use. To compact the database file and reduce disk space, we can use the VACUUM command.

I think it may not be necessary to vacuum the database after every remove command, but it would make sense to call it after removeAllData.

More information about vacuuming can be found at https://blog.eidinger.info/keep-your-coredata-store-small-by-vacuuming#heading-what-is-vacuum.

Let me know if you think it's a good idea, and I'll open a PR.

This isn't very common but if you have a directory such as Database which contains three folders Events, Notes, and Reminders, calling allKeys() on Database will return the sub-databases Events, Notes, and Reminders as keys. While this is technically correct it's a bit unintuitive to how a user would expect this library to work so it would be best to fix that behavior by excluding folders when querying for keys.

Currently if you want to use await to read or write data in a StorageEngine, for exemple a URLSession or CloudKit based storage for a rest api, you can't do it since none of them are marked async, why not provide async variant or juste use async for everything ?

At the moment the implementation of CacheKey is optimized for keys that are file-system safe, accomplished by hashing the CacheKey's String parameter into a String that's safe to save on the file system. As we add StorageEngines (such as SQLiteStorageEngine) that hashing becomes unnecessary, especially since that calculation carries a decent amount of overhead.

It would be possible to accomplish this today by adding a generic constraint to every method in the StorageEngine protocol like so, but that seems to add a decent amount of complexity.

func write<T: Key>(_ data: Data, key: Key) throws

Instead what I propose is to save this change for when Swift 5.7 is released, to have a simple type signature like this.

func write(_ data: Data, key: any Key) throws

The main task is to deprecate CacheKey, create a new Key protocol, and two Key implementers, one focused on the file system, and a generic key that just stores the input String as the Keys value.

We would create a new Key protocol like so.

public protocol Key: Codable, Equatable, Hashable {

    /// The `String` representation of your `Key`.
    var value: String { get }

}

Deprecate the CacheKey type (and conform it to Key for people still using it).

@available(*, deprecated, renamed: "FileSystemSafeStorageKey")
public struct CacheKey: Key {

    public let value: String

    public init(url: URL) {
        let md5HashedURLString = Self.sanitizedURLString(url).md5
        self.init(verbatim: md5HashedURLString.uuidFormatted ?? md5HashedURLString)
    }

    public init(_ value: String) {
        self.init(verbatim: value.md5.uuidFormatted ?? value.md5)
    }

    public init(verbatim value: String) {
        self.value = value
    }

}

Create a new FileSystemSafeStorageKey which is what you will use with DiskStorage instead of CacheKey.

public struct FileSystemSafeStorageKey: Key {

    // … All of the contents of `CacheKey`

}

And a new StorageKey that allows you to use a String as is, with no hashing. This will be what SQLiteStorageEngine uses, and what other StorageEngines will likely default to as well.

public struct StorageKey: Key {

    /// The `String` representation of your `StorageKey`.
    public let value: String

    /// Initializes a `StorageKey` from a `String`, creating a hashed version of the input `String`.
    /// This initializer is useful if you plan on using `StorageKey`s for storing files on disk
    /// because file have many limitations about characters that are allowed in file names,
    /// and the maximum length of a file name.
    /// - Parameter value: The `String` which will serve as the underlying value for this `CacheKey`.
    public init(_ value: String) {
        self.value = value
    }

    public init(url: URL) {
        self.init(url.absoluteString)
    }

}

extension StorageKey: ExpressibleByStringLiteral {

    public init(stringLiteral value: StringLiteralType) {
        self.init(value)
    }

}

Warnings begin to appear about CacheKey and FileManager.Directory being shared across actor boundaries even though they're not marked as Sendable when you enable these flags.

swiftSettings: .unsafeFlags(["-Xfrontend", "-warn-concurrency", "-Xfrontend", "-enable-actor-data-race-checks"])

These warnings aren't time-critical to fix but will need to be addressed in the future.

Bodega has an explicit version number dependency:

.package(url: "https://github.com/stephencelis/SQLite.swift.git", exact: Version(0, 13, 3)),

and Amplify has an explicit dependency:

.package(url: "https://github.com/stephencelis/SQLite.swift.git", exact: "0.13.2"),

which makes them incompatible, and now I need to have both in my project.

Any way to change this on Bodega's side? On the side of AWS this is a known issue: aws-amplify/amplify-swift#2592

I'm curious why there isn't an option for int cache key support? From what I've inspected your using sqlite as a basic key value store. We've found that string keys of uuids use a lot of storage space relative to their blob equivalents. And if you want to use uuids as the basis of your key system, you could store them directly as 128bit blob objects in the sql database and save a lot of space.

We did a test for our own db recently and it reduced our DB 4x in size by replacing our string UUID keys to int / blob equivalents.

It would be really powerful if we would have an Int option for cacheKeys and/or an optimized version for UUIDs. FileStore / JSONStore could use the string representation, but sqllite and maybe others would use the int / uuid representations directly. This would then bubble up to Boutique

https://stackoverflow.com/questions/11337324/how-to-efficient-insert-and-fetch-uuid-in-core-data/11337522#11337522

Hi @mergesort I've implemented a CloudKit backend for Bodega here (https://github.com/binaryscraping/cloudkit-bodega).

But as it's remote storage, there are some non-ideal parts, such as the fetch of all data and keys, as depending on the amount of data, can be a huge problem.

For CloudKit, I've implemented this by looping through the cursors until all items are fetches, but ideally would be better if Bodega provided some way of paginating the results, then the caller could request the next page whenever it needs, and we don't need to fetch everything upfront.

 private func fetchAllRecords() async throws -> [(CKRecord.ID, Result<CKRecord, Error>)] {
    let query = CKQuery(recordType: recordType, predicate: NSPredicate(value: true))
    var results: [(CKRecord.ID, Result<CKRecord, Error>)] = []
    var (matchResults, queryCursor) = try await privateDatabase.records(matching: query)
    results.append(contentsOf: matchResults)

    while let cursor = queryCursor {
      (matchResults, queryCursor) = try await privateDatabase.records(continuingMatchFrom: cursor)
      results.append(contentsOf: matchResults)
    }

    return results
  }

It would be very helpful to include a guide for how to build your own StorageEngine, rather than advising people to build their own based on the bundled StorageEngines.

There's no good way to implement this correctly, because there's no way for users to know whether specific passed-in keys have been skipped due to non-existence.

For example:

let keys = [goodKey1, badKey2, badKey3, goodKey4, badKey5, goodKey6]
let data = await storage.read(keys: keys)

// data contains three values, but for which keys?

When moving some dependencies from Cocoapod to Swift package, the Bodega SQLiteStorageEngine started failing to initialize. Initially I thought it was due to some SQLite incompatibility with the dependency and Bodega, but I was able to reproduce by removing the dependency entirely.

One thing stuck out though: the SQLite.Connection error indicated that the database at the path was missing, but the app container had not been removed, and the database had previously worked as expected.

    public init?(directory: FileManager.Directory, databaseFilename filename: String = "data") {
        self.directory = directory

        do {
            if !Self.directoryExists(atURL: directory.url) {
                try Self.createDirectory(url: directory.url)
            }

            self.connection = try Connection(directory.url.appendingPathComponent(filename).appendingPathExtension("sqlite3").absoluteString)
            self.connection.busyTimeout = 3

            try self.connection.run(Self.storageTable.create(ifNotExists: true) { table in
                table.column(Self.expressions.keyRow, primaryKey: true)
                table.column(Self.expressions.dataRow)
                table.column(Self.expressions.createdAtRow, defaultValue: Date())
                table.column(Self.expressions.updatedAtRow, defaultValue: Date())
            })
        } catch {
            return nil
        }
    }

directory.url would pass the fileExists() check, however when I would use FileManager to test the appended database filename URL, it would fail.

(lldb) po FileManager.default.fileExists(atPath: directory.url.appending(path: filename).appendingPathExtension("sqlite3").absoluteString)
false
(lldb) po FileManager.default.fileExists(atPath: directory.url.appending(path: filename).appendingPathExtension("sqlite3").path)
true

Note that the absoluteString does not exist, but path does exist. When the code is changed to use URL.path the database file is found, SQLite.Connection inits, and the SQLStorageEngine inits.

self.connection = try Connection(directory.url.appendingPathComponent(filename).appendingPathExtension("sqlite3").path)

My recommendation is that use of URL.absoluteString is removed and replaced with URL.path and URL.path() for iOS 16 (and other OS versions).

SQLiteSwift is doing rather minimal work in the SQLiteStorageEngine, and was added because I was building a prototype to see the performance of an SQLite-backed StorageEngine. Now that the concept has been proven out quite successfully I'd like to remove the dependency, though I don't have the time to do so at the moment so Bodega can be a free-standing solution without depending on any other libraries.

I'd be very happy to accept help if someone has the expertise to convert the handful of queries that make up the read/write/delete operations in SQLiteStorageEngine, and willing to offer help if you run into any issues. 🙇🏻‍♂️