Change crypto store transaction API #582

dbkr · 2017-11-29T16:23:58Z

To allow multiple things to be fetched/stored in a single
transaction.

Currently it is still just the account that's actually in
indexeddb though.

To allow multiple things to be fetched/stored in a single transaction. Currently it is still just the account that's actually in indexeddb though.

richvdh · 2017-11-29T17:10:04Z

package.json

@@ -15,7 +15,7 @@
    "build": "babel -s -d lib src && rimraf dist && mkdir dist && browserify -d browser-index.js | exorcist dist/browser-matrix.js.map > dist/browser-matrix.js && uglifyjs -c -m -o dist/browser-matrix.min.js --source-map dist/browser-matrix.min.js.map --in-source-map dist/browser-matrix.js.map dist/browser-matrix.js",
    "dist": "npm run build",
    "watch": "watchify -d browser-index.js -o 'exorcist dist/browser-matrix.js.map > dist/browser-matrix.js' -v",
-    "lint": "eslint --max-warnings 109 src spec",
+    "lint": "eslint --max-warnings 102 src spec",


richvdh

yup, looks entirely sane to me. I think this API is going to work well. At least as well as can be expected given the indexdb/promise mismatch.

A few nitpicks/suggestions.

richvdh · 2017-11-29T17:20:14Z

src/crypto/OlmDevice.js

 *
+ * @param {*} txn


could you document this?

richvdh · 2017-11-29T17:21:58Z

src/crypto/OlmDevice.js

-                const pickledAccount = account.pickle(this._pickleKey);
-                save(pickledAccount);
-            });
+            func(account);


should we not return the result here and at line 194?

Possibly, but as per other comment it could get quite hairy, so probably simpler to just never do it at all.

richvdh · 2017-11-29T17:22:06Z

src/crypto/OlmDevice.js

 };

+OlmDevice.prototype._storeAccount = function(txn, account) {


some docs please

richvdh · 2017-11-29T17:23:37Z

src/crypto/OlmDevice.js

@@ -270,9 +268,13 @@ OlmDevice.prototype._getUtility = function(func) {
 * @return {Promise<string>} base64-encoded signature
 */
 OlmDevice.prototype.sign = async function(message) {
-    return await this._getAccount(function(account) {
-        return account.sign(message);
+    let result;


again, can't we pass the result up through the callbacks to save us having to fiddle with a local result?

(perhaps it actually ends up being more confusing?)

we can, but it could now be quite a few callbacks, and you could be doing more than one get in the transaction, at which point it's not clear which one the transaction promise would return, so I think I'd vote for just making you assign it to a captured variable.

the transaction promise would return whatever is returned by the func passed to doTxn.

I don't think I agree with you, but I don't feel super strongly about it.

richvdh

lgtm

[Full Changelog](matrix-org/matrix-js-sdk@v0.9.2...v0.10.0-rc.1) * Fix duplicated state events in timeline from peek [\matrix-org#630](matrix-org#630) * Create indexeddb worker when starting the store [\matrix-org#627](matrix-org#627) * Fix indexeddb logging [\matrix-org#626](matrix-org#626) * Don't do /keys/changes on incremental sync [\matrix-org#625](matrix-org#625) * Don't mark devicelist dirty unnecessarily [\matrix-org#623](matrix-org#623) * Cache the joined member count for a room state [\matrix-org#619](matrix-org#619) * Fix JS doc [\matrix-org#618](matrix-org#618) * Precompute push actions for state events [\matrix-org#617](matrix-org#617) * Fix bug where global "Never send to unverified..." is ignored [\matrix-org#616](matrix-org#616) * Intern legacy top-level 'membership' field [\matrix-org#615](matrix-org#615) * Don't synthesize RR for m.room.redaction as causes the RR to go missing. [\matrix-org#598](matrix-org#598) * Make Events create Dates on demand [\matrix-org#613](matrix-org#613) * Stop cloning events when adding to state [\matrix-org#612](matrix-org#612) * De-dup code: use the initialiseState function [\matrix-org#611](matrix-org#611) * Create sentinel members on-demand [\matrix-org#610](matrix-org#610) * Some more doc on how sentinels work [\matrix-org#609](matrix-org#609) * Migrate room encryption store to crypto store [\matrix-org#597](matrix-org#597) * add parameter to getIdentityServerUrl to strip the protocol for invites [\matrix-org#600](matrix-org#600) * Move Device Tracking Data to Crypto Store [\matrix-org#594](matrix-org#594) * Optimise pushprocessor [\matrix-org#591](matrix-org#591) * Set event error before emitting [\matrix-org#592](matrix-org#592) * Add event type for stickers [WIP] [\matrix-org#590](matrix-org#590) * Migrate inbound sessions to cryptostore [\matrix-org#587](matrix-org#587) * Disambiguate names if they contain an mxid [\matrix-org#588](matrix-org#588) * Check for sessions in indexeddb before migrating [\matrix-org#585](matrix-org#585) * Emit an event for crypto store migration [\matrix-org#586](matrix-org#586) * Supporting fixes For making UnknownDeviceDialog not pop up automatically [\matrix-org#575](matrix-org#575) * Move sessions to the crypto store [\matrix-org#584](matrix-org#584) * Change crypto store transaction API [\matrix-org#582](matrix-org#582) * Add some missed copyright notices [\matrix-org#581](matrix-org#581) * Move Olm account to IndexedDB [\matrix-org#579](matrix-org#579) * Fix logging of DecryptionErrors to be more useful [\matrix-org#580](matrix-org#580) * [BREAKING] Change the behaviour of the unverfied devices blacklist flag [\matrix-org#568](matrix-org#568) * Support set_presence=offline for syncing [\matrix-org#557](matrix-org#557) * Consider cases where the sender may not redact their own event [\matrix-org#556](matrix-org#556)

Change crypto store transaction API

f11a58e

To allow multiple things to be fetched/stored in a single transaction. Currently it is still just the account that's actually in indexeddb though.

dbkr assigned richvdh Nov 29, 2017

delint

fece506

richvdh reviewed Nov 29, 2017

View reviewed changes

richvdh assigned dbkr and unassigned richvdh Nov 29, 2017

Doc some more things

ecc1c86

dbkr assigned richvdh and unassigned dbkr Nov 29, 2017

richvdh approved these changes Nov 29, 2017

View reviewed changes

richvdh assigned dbkr and unassigned richvdh Nov 29, 2017

dbkr merged commit 082683b into develop Nov 30, 2017

t3chguy deleted the dbkr/crypto_store_txn_api branch May 10, 2022 14:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change crypto store transaction API #582

Change crypto store transaction API #582

dbkr commented Nov 29, 2017

richvdh Nov 29, 2017

richvdh left a comment

richvdh Nov 29, 2017

dbkr Nov 29, 2017

richvdh Nov 29, 2017

dbkr Nov 29, 2017

richvdh Nov 29, 2017

dbkr Nov 29, 2017

richvdh Nov 29, 2017

dbkr Nov 29, 2017

richvdh Nov 29, 2017

richvdh left a comment

		};

		OlmDevice.prototype._storeAccount = function(txn, account) {

Change crypto store transaction API #582

Change crypto store transaction API #582

Conversation

dbkr commented Nov 29, 2017

Choose a reason for hiding this comment

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richvdh left a comment

Choose a reason for hiding this comment