js-ceramic
js-ceramic copied to clipboard
fix(http-client): Remove streamcache from http client
The "streamCache" doesn't actually act as a cache. Every time we load a stream, we still make a request to the Ceramic node, even if the stream is in the cache. So it's not actually saving us any i/o.
One thing the cache is doing is keeping different stream handles that were loaded at different times in sync with the most recently loaded state. I would argue this is undesirable and confusing behavior as it means that the current state of a stream instance can change out from under an application without it knowing. When an app wants to ensure they have the most recent state in their stream handle, they already have the option of calling sync()
or subscribe()
on the stream handle. But unless the app does one of those two things, the state of the handle should remain fixed as it was when the data was loaded from the server (which is already the behavior of the core
client, it's currently inconsistent).
The other thing the cache did was keeping track of stream handles that have been given out so that if they have any subscriptions they can be closed when the http client is closed. That is lost in this PR so apps that explicitly subscribe to streams will need to take care to close their own subscriptions. Not ideal, but not terrible either, and something we'll likely have to revisit when we do the proper subscription API anyway. If you think auto-closing subscriptions when the http-client closes is important behavior to preserve I can try to preserve it, I had an almost-working PR to do that but hit some weird RXJS behaviors that I couldn't figure out how to fix.
This fixes a bug (https://github.com/ceramicnetwork/js-ceramic/issues/2353) where loading a stream at a specific CommitID could change the state out from under all existing stream handles to that stream to reset the state to that CommitID
NET-1691 loadStream is not re-entrant
https://github.com/ceramicnetwork/js-ceramic/issues/2353
Describe the bug When running multiple calls to loadStream in parrallel, only the tip document is returned.
See discussion here: https://forum.ceramic.network/t/solved-how-to-load-prior-commits/117
To Reproduce Steps to reproduce the behavior: Execute the following script:
import { Core } from '@self.id/core'
import { TileDocument } from '@ceramicnetwork/stream-tile'
import type { JWE } from 'did-jwt';
const did = "did:3:kjzl6cwe1jw146vyxh3bf1l325qcb3b4rxnns181whfw1mezrpmgyia2mm2otzm";
const aliases = JSON.parse(`{
"definitions": {
"AccountDetails": "kjzl6cwe1jw149gym1m5qjzzqjdlupnsxivgeef9z6xjkc14ldbkgwag1cssryr"
},
"schemas": {
"jwe": "ceramic://k3y52l7qbv1frxwvp66vqpcd046ju9ocvimye6htwucpg00h08nltj1fkns6p9ji8"
},
"tiles": {}
}`)
const core = new Core({ ceramic: 'testnet-clay', aliases});
export async function getOtherHistory() : Promise<JWE[]|null>{
console.log(`Loading history for ${did}`);
const { ceramic, dataModel, dataStore } = core;
const definitionId = dataModel.getDefinitionID("AccountDetails");
// The recordId is the stream defined by definition for this DID
const streamId = await dataStore.getRecordID(definitionId!, did);
if (!streamId)
return null;
const stream = await TileDocument.load(ceramic, streamId);
const ids = stream.allCommitIds;
console.log("All Ids:\n", ids.map(id => id.toString()).join('\n'));
console.log(`Found ${ids.length} commits`);
// TODO: do this with a multi-query instead
const commits = await Promise.all(ids.map(id => ceramic.loadStream(id)));
return commits.map(c => c.content as JWE);
}
const all = await getOtherHistory();
if (all) {
for (const commit of all) {
console.log(commit.ciphertext);
}
}
You'll see 20 different instances of identical ciphertext
Expected behavior About 20 different commits should be logged with unique ciphertext
Screenshots
Ceramic versions Latest
Machine, OS, browser information (please complete the following information): Windows
Additional context
From the discussion:
Internally, streams are cached and the call to loadStream syncs stream to the given ID. Doing them all at once means that once complete, the single cache is still sync’ed on a single commit, and we get back X documents all referencing that single (tip) commit.
The underlying issue is that we treat a snapshot as a fully formed stream instance. Maybe instead we could internally handle requests with streamIDs and commitIDs separately: if commitId is passed, do not put it onto cache.
That actually will bring http client behaviour on par with Ceramic Core.
That actually will bring http client behaviour on par with Ceramic Core.
Even without using CommitIDs there is a behavior difference between the http client and Ceramic core. In Ceramic core, each stream instance in app code is independent of other stream instances. Loading one instance with newer state will not change the state out from under existing instances. For existing instances to get newer state, they either need to call .sync()
or .subscribe()
. This is the desired behavior in my opinion. In the HTTP client, if one instance of a stream syncs newer state, all stream instances sync that newer state. This means the state of a stream instance change without the app even doing anything to that stream instance. This is confusing behavior in my opinion that should be removed. I wrote a test to demonstrate the behavior difference between core and client here: https://github.com/ceramicnetwork/js-ceramic/pull/2372
Ping @ukstv, this is getting in the way of testing the PR I am working on now
Ugh, I am convinced this is all right.
@stbrody Might I ask you to push something here to re-trigger the checks, please? They should be green: https://github.com/ceramicnetwork/js-ceramic/actions/runs/3298887592