Page MenuHomePhabricator
Differential D11300

[sqlite] add methods to operate on integrity store table
ClosedPublic
Actions

Authored by will on Mar 11 2024, 6:36 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 15, 1:03 PM
Unknown Object (File)
Apr 6 2024, 12:09 PM
Unknown Object (File)
Apr 5 2024, 4:59 AM
Unknown Object (File)
Apr 4 2024, 1:40 PM
Unknown Object (File)
Apr 3 2024, 2:32 PM
Unknown Object (File)
Apr 3 2024, 1:32 AM
Unknown Object (File)
Apr 1 2024, 11:07 PM
Subscribers
ashoat

Details

Reviewers
kamil
tomek
Commits
rCOMMc12a268115b2: [sqlite] add methods to operate on integrity store table
Summary

Adds methods to operate on integrity store table

Depends on D11323

Test Plan

Successfully ran tests in communities-queries.test.js.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

will created this revision.Mar 11 2024, 6:36 AM
Herald added a subscriber: ashoat. · View Herald TranscriptMar 11 2024, 6:36 AM
Harbormaster completed remote builds in B27432: Diff 37980.Mar 11 2024, 7:15 AM
will requested review of this revision.Mar 11 2024, 7:15 AM
will added a child revision: D11306: [sqlite] implement C++ integrity store ops.Mar 11 2024, 2:04 PM
will edited the summary of this revision. (Show Details)
will added a parent revision: D11298: [lib] Refactor integrity reducer to process threadStore ops with sqlite ops.
kamil accepted this revision.Mar 12 2024, 10:07 AM

Looks good, there is one problem with thread_hash type - but let's discuss this in D11247 and then you can just update the type here

lib/ops/integrity-store-ops.js
38 ↗(On Diff #37980)

why do we have to rename it?

native/cpp/CommonCpp/DatabaseManagers/entities/IntegrityThreadHash.h
11 ↗(On Diff #37980)

this couldn't be int - see my comment here: https://phab.comm.dev/D11247#inline-68347

web/database/queries/integrity-queries.test.js
39–41 ↗(On Diff #37980)

we have a bit different error handling now - see here

82–87 ↗(On Diff #37980)

I guess you can just do something like:

expect(threadHashesFromDB).toStrinctEqual({...TEST_THREAD_HASHES_1, ....TEST_THREAD_HASHES_UPDATE});

what do you think?

This revision is now accepted and ready to land.Mar 12 2024, 10:07 AM
ashoat added inline comments.Mar 13 2024, 9:37 AM
native/ios/Comm.xcodeproj/project.pbxproj
139 ↗(On Diff #37980)

Usually when we see changes to project.pbxproj, we also expect to see some changes to a CMakeLists.txt on Android. Was this tested on Android? Are you sure we don't need to modify any CMakeLists.txt files?

will edited the summary of this revision. (Show Details)Mar 13 2024, 11:36 PM
will edited the summary of this revision. (Show Details)
will added a parent revision: D11323: [lib] return ops from integrity store reducer.
will planned changes to this revision.Mar 14 2024, 9:24 PM
will added inline comments.Mar 18 2024, 9:18 PM
lib/ops/integrity-store-ops.js
38 ↗(On Diff #37980)

Mainly did this for consistency with the type IntegrityStoreOperation. This should've be done when it was first introduced. I'll rebase and remove the need for a rename.

native/ios/Comm.xcodeproj/project.pbxproj
139 ↗(On Diff #37980)

I was going off of the community and keyserver sqlite migration diffs and I don't think we made any changes there.

The CMakeLists.txt for android contains parent folders for mutual code for native:

../../cpp/CommonCpp/
../../cpp/CommonCpp/NativeModules
../../cpp/CommonCpp/NativeModules/InternalModules
../../cpp/CommonCpp/NativeModules/PersistentStorageUtilities
../../cpp/CommonCpp/NativeModules/PersistentStorageUtilities/BackupOperationsUtilities
../../cpp/CommonCpp/NativeModules/PersistentStorageUtilities/DataStores
../../cpp/CommonCpp/NativeModules/PersistentStorageUtilities/ThreadOperationsUtilities
../../cpp/CommonCpp/NativeModules/PersistentStorageUtilities/MessageOperationsUtilities
../../cpp/CommonCpp/NativeModules/PersistentStorageUtilities/MessageOperationsUtilities/MessageSpecs
../../cpp/CommonCpp/DatabaseManagers
../../cpp/CommonCpp/Notifications
../../cpp/CommonCpp/Notifications/BackgroundDataStorage

I tested on android and the app seems to be working, although I haven't thoroughly tested the database

will updated this revision to Diff 38151.Mar 18 2024, 9:19 PM

review feedback

This revision is now accepted and ready to land.Mar 18 2024, 9:19 PM
will marked an inline comment as done.Mar 18 2024, 9:23 PM
will added inline comments.
web/database/queries/integrity-queries.test.js
39–41 ↗(On Diff #37980)

Included in latest rebase

82–87 ↗(On Diff #37980)

Looks good. Included in latest rebase

Harbormaster completed remote builds in B27571: Diff 38151.Mar 18 2024, 9:35 PM
will marked an inline comment as done.Mar 18 2024, 9:43 PM
will added inline comments.
native/cpp/CommonCpp/DatabaseManagers/entities/IntegrityThreadHash.h
11 ↗(On Diff #37980)

addressed in latest rebases

will removed a child revision: D11306: [sqlite] implement C++ integrity store ops.Mar 25 2024, 1:38 PM
will added a child revision: D11306: [sqlite] implement C++ integrity store ops.Mar 25 2024, 1:41 PM
will removed a parent revision: D11298: [lib] Refactor integrity reducer to process threadStore ops with sqlite ops.
Closed by commit rCOMMc12a268115b2: [sqlite] add methods to operate on integrity store table (authored by will). · Explain WhyMar 25 2024, 2:11 PM
This revision was automatically updated to reflect the committed changes.
will added a commit: rCOMMc12a268115b2: [sqlite] add methods to operate on integrity store table.
will mentioned this in D11417: [sqlite] add methods to operate on synced metadata table.Mar 28 2024, 8:59 AM
ashoat mentioned this in D11444: [native] Fix up project.pbxproj after yarn cleaninstall.Mar 28 2024, 10:02 AM
ashoat mentioned this in rCOMMa0cab739d3ff: [native] Fix up project.pbxproj after yarn cleaninstall.Mar 28 2024, 7:13 PM

Revision Contents
Changeset List

PathSize
lib/
ops/
30 lines
native/
cpp/
CommonCpp/
DatabaseManagers/
8 lines
7 lines
48 lines
entities/
24 lines
ios/
Comm.xcodeproj/
2 lines
web/
cpp/
15 lines
database/
_generated/
queries/
108 lines
types/
8 lines
shared-worker/
_generated/
queries/
103 lines
types/
8 lines

Diff 38151

View Options

lib/ops/integrity-store-ops.js

Loading...
View Options

native/cpp/CommonCpp/DatabaseManagers/DatabaseQueryExecutor.h

Loading...
View Options

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.h

Loading...
View Options

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp

Loading...
View Options

native/cpp/CommonCpp/DatabaseManagers/entities/IntegrityThreadHash.h

Loading...
View Options

native/ios/Comm.xcodeproj/project.pbxproj

Loading...
View Options

web/cpp/SQLiteQueryExecutorBindings.cpp

Loading...
View Options

web/database/_generated/comm_query_executor.wasm

Loading...
View Options

web/database/queries/integrity-queries.test.js

Loading...
View Options

web/database/types/sqlite-query-executor.js

Loading...
View Options

web/shared-worker/_generated/comm-query-executor.js

Loading...
View Options

web/shared-worker/_generated/comm_query_executor.wasm

Loading...
View Options

web/shared-worker/queries/integrity-queries.test.js

Loading...
View Options

web/shared-worker/types/sqlite-query-executor.js

Loading...