Skip to content

More sharing tests and improvements#129

Merged
mbrandonw merged 8 commits into
cloudkitfrom
more-sharing-tests-and-improvements
Aug 18, 2025
Merged

More sharing tests and improvements#129
mbrandonw merged 8 commits into
cloudkitfrom
more-sharing-tests-and-improvements

Conversation

@mbrandonw
Copy link
Copy Markdown
Member

@mbrandonw mbrandonw commented Aug 17, 2025

With the improvements we made to the testability of sharing in #128 we can now write tests for behavior we added in #126. And it turns out we had a bug in that PR 😬. We are preventing changes being sync'd from CK to the user device when they have read only permissions. We need to bypass permission checks when rows are updated from the sync engine directly.

@mbrandonw mbrandonw requested a review from stephencelis August 17, 2025 14:08
mbrandonw
mbrandonw commented Aug 17, 2025
View reviewed changes
Comment thread Sources/SharingGRDBCore/CloudKit/CloudKitSharing.swift Outdated
mbrandonw
mbrandonw commented Aug 17, 2025
View reviewed changes
Comment thread Sources/SharingGRDBCore/CloudKit/CloudKitSharing.swift
Comment on lines +153 to +154
public func unshare<T: PrimaryKeyedTable>(record: T) async throws
where T.TableColumns.PrimaryKey.QueryOutput: IdentifierStringConvertible {
Copy link
Copy Markdown
Member Author

@mbrandonw mbrandonw Aug 17, 2025

Choose a reason for hiding this comment

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

Added an endpoint that allows people to un-share a record. Could be handy for easy access to this functionality, but people can always fetch the CKShare themselves and interact with the private database.

mbrandonw
mbrandonw commented Aug 17, 2025
View reviewed changes
Comment thread Sources/SharingGRDBCore/CloudKit/Triggers.swift
}
.where {
$0.parentRecordName.is(nil)
!SyncEngine.isSynchronizingChanges()
Copy link
Copy Markdown
Member Author

@mbrandonw mbrandonw Aug 17, 2025

Choose a reason for hiding this comment

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

This is the bug we had. We need to check permissions only when not updating from the sync engine.

mbrandonw
mbrandonw commented Aug 17, 2025
View reviewed changes
Comment thread Tests/SharingGRDBTests/CloudKitTests/AccountLifecycleTests.swift
try await syncEngine.processPendingDatabaseChanges(scope: .private)
try await syncEngine.processPendingRecordZoneChanges(scope: .private)
assertInlineSnapshot(of: syncEngine.container, as: .customDump) {
assertInlineSnapshot(of: container, as: .customDump) {
Copy link
Copy Markdown
Member Author

@mbrandonw mbrandonw Aug 17, 2025

Choose a reason for hiding this comment

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

I made this change across the entire test suite so it's a little noisy. but we have direct access to container in the tests so no need to go through the sync engine.

mbrandonw
mbrandonw commented Aug 17, 2025
View reviewed changes
Comment thread Tests/SharingGRDBTests/CloudKitTests/SharingTests.swift
}

@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
@Test func shareUnshareShareAgain() async throws {
Copy link
Copy Markdown
Member Author

@mbrandonw mbrandonw Aug 17, 2025

Choose a reason for hiding this comment

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

This confirms that a bug reported by @appfrosch is fixed. previously when un-sharing we did clean up the CKShare state from the sync metadata.

@mbrandonw mbrandonw merged commit 0f6e7c4 into cloudkit Aug 18, 2025
2 of 5 checks passed
@mbrandonw mbrandonw deleted the more-sharing-tests-and-improvements branch August 18, 2025 20:34
@mbrandonw mbrandonw added the cloudkit Related to our CloudKit synchronization release label Sep 2, 2025
stephencelis added a commit that referenced this pull request Sep 17, 2025
@stephencelis @mbrandonw
@robfeldmann @Narsail @yimajo @thimic
CloudKit Synchronization + Sharing (#112)
0b11498 
* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* a test for deletion

* Fixes #32 failure to compile in release configuration

* wip

* Fix a reference violation.

* wip

* add test

* wip

* wip

* format/

* wip

* wip

* fix tests

* fix tests

* add a test

* wip

* wip

* wip

* wip

* wip

* simplify

* wip

* Delete records in groups.

* wip

* wip

* wip

* some doc updates

* wip

* update package

* Support zones in mock cloud database.

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* fix dep

* wip

* wip

* Apply suggestion from @stephencelis

Co-authored-by: Stephen Celis <stephen@stephencelis.com>

* Better way to process record/database changes in tests.

* Fail if we process changes without there being any changes.

* merge fix

* fix import

* Allow sync engine to be initalized with a zone.

* wip

* Updates swift-structured-queries

* 'Float16' is unavailable in Mac Catalyst

While Xcode’s error says 'Float16' is unavailable in Mac Catalyst, the docs suggest it is available on Apple silicon, and unavailable on Intel when targeting macOS. Checking for Intel allows archiving for catalyst.

* Account management.

* Handle account lifecycle events.

* wip

* Support account status in mock cloud kit.

* wip

* wip

* Derive metadatabase URL from database URL

* wip

* wip

* wip

* updates to docs

* write test for certain ck scenario

* wip

* Revert "wip"

This reverts commit ba77edd.

* wip

* get rid of uuid zeros

* docs

* wip

* wip

* Public method for fetching metadata.

* Revert "Public method for fetching metadata."

This reverts commit 6519f17.

* Public method for fetching metadata.

* Apply suggestions from code review

* wip

* wip;

* Update Tests/SharingGRDBTests/Internal/BaseCloudKitTests.swift

Co-authored-by: Stephen Celis <stephen@stephencelis.com>

* Update Tests/SharingGRDBTests/CloudKitTests/AccountLifecycleTests.swift

Co-authored-by: Stephen Celis <stephen@stephencelis.com>

* wip

* fix

* fix test

* wip

* wip

* wip

* todos

* Support in-memory metadatabase.

* wip

* wip

* fix merge

* Improve `SyncEngine.init` (#113)

* Improve `SyncEngine.init`

- Infer default cloud container using SwiftData
- Statically require string identifier primary keyed tables

* wip

* wip

* fix test

* wip

* wip

* wip

* Regenerate macro code for CloudKit tables. (#120)

* Regenerate macro code for CloudKit tables.

* formatting

* fix warnings

* fix typo

* wip

* Add Tagged support to SharingGRDB

* Add uniqueness to tag titles. (#119)

* Add uniqueness to tag titles.

* wip

* wip

* wip

* wip

* wip

* wip

* Improve un-sharing records.

* Add Available Permissions (#123)

Co-authored-by: David Moeller <david.moller@sensorfact.nl>

* Make sync engine play nicely with tests (#124)

* wip

* control dates internally

* wip

* wip

* add a test

* wip

* Handling more CloudKit sharing edge cases (#126)

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* clean up

* wip

* wip

* wip

* wip

* Making sharing more testable (#128)

* Beginnings of making sharing more testable.

* wip

* wip

* wip

* wip

* fixes

* wip

* More sharing tests and improvements (#129)

* Fixing more sharing edge cases with tests.

* wip

* wip

* wip

* wip

* dont emit error when unsharing unshared record.

* wip

* fix

---------

Co-authored-by: Stephen Celis <stephen@stephencelis.com>

* Test for generated column behavior. (#136)

* Test for generated column behavior.

* snap

* wip

* Revert examples

* wip

* wip

* wip

* Fix upsert for tables that only have a primary key.

* add some tests

* Add `SyncEngine.{start,stop,isRunning}` (#142)

* Add `SyncEngine.{start,stop}()`

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* Remove unneeded sleeps

* format

---------

Co-authored-by: Brandon Williams <mbrandonw@hey.com>

* CloudKit: Compile tests on non-macOS platforms (#144)

* CloudKit: Compile tests on non-macOS platforms

* Bump GRDB requirement

* wip

* wip

* wip

* Minimize exports

* wip

* wip

* Remove trigger check

* Some permission fixes (#147)

* Fix up some permission loopholes.

* wip

* wip

* clean up

* Document scene delegate methods for accepting shares. (#148)

* Document scene delegate methods for accepting shares.

* Update Sources/SharingGRDBCore/Documentation.docc/Articles/CloudKitSharing.md

---------

Co-authored-by: Stephen Celis <stephen@stephencelis.com>

* Clean up some tests

* Add link to apple docs for deploying schema

* update docs wrt SyncEngine.isSychnronizing

* Delete CK record when updating primary key. (#151)

* Delete CK record when updating primary key.

* wip

* wip

* Validate foreign keys point to synchronized tables. (#152)

* wip

* Lots of updates to docs. (#153)

* Lots of updates to docs.

* db error

* fixes

* Beginning of renaming package to SQLiteData.

* Rename more things.

* wip

* flatten

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* fix test

* wip

* More docs (#154)

* More docs

* wip

* wip

* wip

* wip

* Format SQLiteData entry in README.md table

* wip

* Update Sources/SQLiteData/CloudKit/CloudKitSharing.swift

* Update Sources/SQLiteData/Documentation.docc/Articles/CloudKit.md

* Update Sources/SQLiteData/CloudKit/SyncMetadata.swift

* Format SQLiteData performance table for clarity

---------

Co-authored-by: Stephen Celis <stephen@stephencelis.com>

* Fetch pending record zone changes through metadatabase (#155)

* wip

* docs on uniqueness

* Update README.md

* wip

* Triage some todos (#157)

* Triage todos'

* wip

* wip

* wip

* wip

* wip

* CloudKit docs organization (#158)

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* Add observation to `SyncEngine.isRunning` (#159)

* Add observation to `SyncEngine.isRunning`

* wip

* wip

* wip

* wip

* wip

* fix

* finish test

* Add test for cyclic schemas'

* docs on PK migration

* wip

* docs

* Audit reading from metadatabase (#161)

* Audit metadatabase

* convert more userDatabase to metadatabase

* wip

* wip

* wip

* wip

* format

* wip

* docs

* CloudKit sqlite data pragmas (#162)

* CloudKit: Use virtual table helpers

Let's prefer our builder over SQL strings when it's easy to do so.

* wip

* wip

* wip

* wip

* Move menu into toolbar hstack.

* Fix tests for iOS. (#165)

* Optimize a few sync metadata queries (#169)

Let's select/decode just the data we need.

* More efficient joins to SyncMetadata table (#163)

* Improve tools for joining SyncMetadata table.

* wip

* wip

* wip

* wip

* wip

* wip

---------

Co-authored-by: Stephen Celis <stephen@stephencelis.com>

* Make upsertFromServerRecord async. (#170)

Co-authored-by: Brandon Williams <mbrandonw@hey.com>

* Cloudkit sqlite data send changes background (#167)

* Immediately send pending changes when backgrounding app.

* wip;

* Revert "wip;"

This reverts commit fb68ce7.

* wip

* Revert "Revert "wip;""

This reverts commit 2ddfe3d.

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* Upload unknown records to CloudKit on sign in. (#168)

* Upload unknown records to CloudKit on sign in.

* fix tests

* wip

* wip

* wip

* merge fix

* wip;

* Fix sign out. (#164)

* Fix sign out.

* wip

* fix

* Apply suggestions from code review

* merge fix

---------

Co-authored-by: Stephen Celis <stephen@stephencelis.com>

* fixes

* wip

* wip

* wip

* wip

* wip

* Add `database` for context-sensitive provisioning (#175)

* Add `database` for context-sensitive provisioning

This small helper will take live/test/preview into account to simplify
how folks bootstrap their databases.

* wip

* wip

* Update DefaultDatabase.swift

* wip

* another way to get container identifier

* Modernize SyncUps and add iCloud sync. (#176)

* Modernize SyncUps and add iCloud sync.

* fix tests

* Send sync engine changes on resign foreground. (#177)

* Send changes when resigining active.

* fix

* wip

* Bring back other demo apps. (#178)

* Bring back other demo apps.

* wip

* Rename more references to sqlite-data

* wip

* wip

* wip

* go back to getting cloudkit container identifier from swiftdata

* wip

* A few fixes for Xcode 26. (#180)

* A few fixes for Xcode 26.

* wip

* wip

* Throw error when changing zones for existing record (#179)

* Throw error when moving records between zones.

* wip

* wip

* wip

* wip

* Fix floating point bug (#183)

* Fix floating point comparison bug.

* wip

* wip

* wip;

* wipa

* wip

* wip

* wipa

* Fix Examples/README: project reference (#186)

* Update README.md

- Replace the removed `SharingGRDB.xcworkspace` reference with `Examples.xcodeproj`.
- Explain that each example app is worked on individually by selecting its scheme in Xcode.

* Update README.md

* Perform multiple upserts in a single transaction. (#185)

* Perform multiple upserts in a single transaction.

* wip

* wip

* wip

* wip

* Only reference UIApplication on iOS (#190)

* Modernize reminders (#184)

* Modernize reminders with triggers.

* wip;

* wip

* wip;

* wip

* wip

* wip

* Update Examples/Reminders/Schema.swift

* fix tests

---------

Co-authored-by: Stephen Celis <stephen@stephencelis.com>

* Add zone to sync metadata (#187)

* Store zone data in sync metadata.

* wip

* wip

* wip

* wip

* wip

* wip

* Support moving records across zones (#188)

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* clean up

* fix snapshot

* wip

* wip

* wip

* wip

* support moving shares between zones and receiving saves before deletes

* wip

* Update Sources/SQLiteData/CloudKit/Internal/Triggers.swift

* wip

* wip

* wip

* format

* wip;

---------

Co-authored-by: Stephen Celis <stephen@stephencelis.com>

---------

Co-authored-by: Stephen Celis <stephen@stephencelis.com>

* wip

* Fix zone deletion (#192)

* wip

* wip

* wip

---------

Co-authored-by: Brandon Williams <135203+mbrandonw@users.noreply.github.com>
Co-authored-by: Brandon Williams <mbrandonw@hey.com>
Co-authored-by: Rob Feldmann <me@robfeldmann.com>
Co-authored-by: David Moeller <david_moeller@me.com>
Co-authored-by: David Moeller <david.moller@sensorfact.nl>
Co-authored-by: Yoshinori Imajo <y.imajo@gmail.com>
Co-authored-by: Michael Thingnes <thimic@users.noreply.github.com>
mackoj pushed a commit to mackoj/sqlite-data that referenced this pull request Dec 5, 2025
@stephencelis
Update SelectStatements.md (pointfreeco#130)
1b9d608 
Fixes pointfreeco#129.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stephencelis stephencelis stephencelis approved these changes

Assignees

No one assigned

Labels

cloudkit Related to our CloudKit synchronization release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mbrandonw @stephencelis