Proposal: Giving control of session state to app developers

While I like the original proposal for it’s simplicity, I would have to lean towards @wbobeirne simply because we have to know when the state (within the storage) has to be updated!

Otherwise, if we were to keep the Original Proposal then there’d have to be get/set callbacks or something like an update callback within the AppConfig like so:

class AppConfig {
  // ...
  /**
   * If `value` is null, delete, otherwise, set to item of index `key` the new `value`
   * @type {(state: BlockstackSession, key: string, value?: any) => Promise<void>}
  **/
  updateSession: null
}

I think that breaking changes are fine, but would be more “palatable” if we also implemented the code splitting change within the same update. However that makes the task very big and slows down implementing this to a full stop, so I really don’t know.

Otherwise, I love the simplicity of the api of putProfile and putFile… it isn’t fun to do this instead, as I have to right now…

I really like Will’s idea of having storage strategies that default based on the environment.

1 Like

Besides using them for mocking, are there any desirable storage strategies that are not Gaia? Serious question.

These are storage strategies for the session state, not for app data. We currently use localStorage

1 Like

If we’re moving ahead with this proposal, can I request that we make the authenticator URL configurable in the BlockstackSession object?

This would make it possible to build automated browser tests for the auth flow by redirecting auth to a deploy preview build. And this also enable people to build their own authenticators in the future.

class BlockstackSession { 
   ...
  authenticatorURL: ?string
}
3 Likes

I think this makes a lot of sense.


It’s actually already configurable in our shipped versions here too:

export function redirectToSignInWithAuthRequest(authRequest: string = makeAuthRequest(),
                                                blockstackIDHost: string =
                                                DEFAULT_BLOCKSTACK_HOST)
1 Like

I don’t like the proposed API. This should be as simple as possible with few steps.

I was putting off replying because I wanted to come up with an actual alternative. But the gist of my idea was to actually split into “high level API” and “low level API”. Where some functionality does not exist from the high level API (like if you want to do custom things), but most does.

Will’s idea of using the existing API sounds good, because it already exists. The default Storage strategy selection can be done based on whether it runs in browser or Node. That sounds like a common enough thing to make easy. Or possible better (at least at first until we see how it’s used) just default to LocalStorage (browser), and make People use the low level API for setting another storage provider.

I’m basically thinking along the lines of how Python requests work, where it gives you a standard Session if you just use it. But allows you to custom-create your own session if you want. See how request.get is implemented (tiny bit simplified);

# call: request.get(args)
s = request.Session()
return s.get(...args)

If you ever wanted to do something advanced, you’d just do that on your own:

mysess = request.Session(lots=of, options=Here())
response = mysess.get(...)  # yes, session also has the developer friendly .get/.post helpers

Sorry I never got a clear idea how that would look for the Blockstack API. But I think it’d be a mix between Larry and Will’s API.

Agree - Work on this is on a bit of hold at the moment while I work on some other things. I’m leaning towards merging the classes I Blockstack and BlockstackSession into one class. An instance of that class represents a logged user session. It could be called Session or User or Identity or something to that effect.

I’m not proposing making any changes to the various public API methods except that some would become instance methods.

I really like Will’s storage strategy proposal as well.

I do think you could make it a little simpler by creating a new “Blockstack” object via

const blockstack = new Blockstack({
  appConfig: { appDomain: myAppDomain },
  sessionConfig: { authenticatorUrl: 'localhost:8700' }
});

and then just use some Object.assign's to assign defaults, etc. (do note it’s not recursive so you’d have to do some for..in stuff).

In addition, instead of passing the session within each action (I think that’s pretty complicated), here’s an example for using a callback instead:

// web / localstorage
const onSessionUpdate = (state, key, value) => {
  if(!state) window.localStorage.clear();
  else if(!key) Object.keys(state).forEach(k => window.localStorage.setItem(k, state[k]);
  else if(!value) window.localStorage.removeItem(key);
  else window.localstorage.setItem(key, value);
};
// custom rethinkdb wrapper
const onSessionUpdate = (state, key, value) => {
  if(!state) db.run(stateTable.delete());
  else if(!key) db.run(stateTable.replace(state));
  else if(!value) db.run(stateTable.get(key).delete());
  else {
    const obj = {};
    obj[key] = value;
    db.run(stateTable.insert(obj, { conflict: 'update' }));
  }
};

And then using them in the config…

const blockstack = new Blockstack({
  appConfig: { appDomain: 'my-domain' },
  sessionConfig: { updateCallback: onSessionUpdate }
});

But outside of those specifics, the biggest difference I see between the Original Proposal and Will’s would be the former uses the storage provider as something to backup and recover the state, whereas the latter uses it as a complete storage solution. I feel like the former is a little easier in regards to speed and ease-of-use (for the library at least), but the latter has better persistence in regards to crashing and unhandled closing of the application. You could also use the latter along with flux/redux/vuex instead of localstorage as well.

Regardless, with either being done there should be a default driver to use localstorage, and it would have to be overidden to use a custom storage backend (like localForage or a db).


Until either solution gets implemented though, here’s a quick fix for those who are running on node currently:

1 Like

Thanks to everyone for your feedback!

I’ve opened a pull request that implements a lot of was discussed in this topic.

It moves user-session related calls into the instance of an object UserSession. Data related to a given user-session is encapsulated in this instance and persisted by an instance of a object that extends SessionDataStore.

Defaults such as app domain, etc continue to be auto-detected when run in a browser environment.

This means the library can continue to be used with minimal configuration in a web browser environment:

import { UserSession } from 'blockstack'

const userSession = new UserSession()

userSession.redirectToSignIn()

userSession.getFile('file.json')

In a web browser, by default UserSession will store session data in the browser’s localStorage through an instance of LocalStorageStore.

This means that app developers can leave management of session state to users.

In a non-web browser environment, it is necessary to pass in an instance of AppConfig which defines the parameters of the current app.

  import { AppConfig, UserSession } from 'blockstack'
  const appConfig = new AppConfig('http://localhost:3000')
  const userSession = new UserSession({ appConfig })

In non-browser environments (ie. node), session state is stored in the instance using the InstanceDataStore. If a developer wishes to store this elsewhere, he should can extend SessionDataStore and implement his own storage strategy.

You can test this by linking it to this branch of the blockstack todos app: https://github.com/blockstack/blockstack-todos/tree/feature/blockstack-19

Here are screenshots of docs for the UserSession API:


The pull request requires additional testing, test coverage and documentation before merging. I wanted to share it earlier rather than later to give everyone an opportunity to share your thoughts. I’d love your feedback either here or in the pull request!

2 Likes

Looks good to me, @larry! A couple quick questions:

  • Is this PR intended to be backwards-compatible with master, or will existing apps need to be patched to use UserSession and AppConfig objects?

  • I see that the UserSession object also contains the storage API. Do we want to continue including storage and authentication methods in the same facade object, or would this be a good opportunity to begin splitting them out (i.e. pending the development of gaia.js)?

  • To be clear, the new UserSession and AppConfig objects remove all dependencies on globals, right? i.e. Will it be safe to instantiate and use multiple UserSession and AppConfig objects in the same program?

2 Likes

Will it be safe to instantiate and use multiple UserSession and AppConfig objects in the same program?

This is something Stealthy will need regardless of whether it’s officially supported. The use case is multiple Blockstack dApps operating within Stealthy Mobile and writing to their respective GAIA buckets AND other dApp GAIA buckets.

— Edit —
To be clear, these dApps will be logging in via association Token and their code will be actively reading/writing from/to their GAIA, but they’ll also have an API to exchange data with Stealthy’s environment which will also be able to read/write Stealthy GAIA. (This addresses your point regarding safety–Stealthy doesn’t want to get into the business of writing to other peoples GAIA–that was enough of a nightmare bug when it happened with the SDKs logging out of one ID and into another).

@alexc.id would something like the Multiple Blockstack App Login Integration help for what you need, then?

I’m trying to wrap my head around how it would be implemented with accordance to this refactor, but I suppose it would have to be done within multiple UserSessions/AppConfigs (though only through one auth request, eh?).

1 Like

@MichaelFedora, sorry for the late reply. Yes, we need something like multiple app login or the ability to add an app login token / session within an exiting dApp session.

I’m trying to push for discussion on that subject in this forum post, Do You Own Your Data If You Can’t Control Or Access It, and I’m going to add it to the engineering meeting agenda for this week.

1 Like

No, it is not backwards compatible.

I’d suggest doing that in a future release.

It should be safe to use multiple instances of these objects in the same program.

AppConfig has a default argument that is the window.location.origin global which allows use on browsers without specifying the app domain parameter. On non-browser platforms, one should specify the parameter.

The pull request implementing this, https://github.com/blockstack/blockstack.js/pull/542, is ready for review.

It is released on npm as blockstack v19.0.0-alpha.3 for ease of testing.

To see this in action, try out the animal kingdom sample app: https://github.com/larrysalibra/animal-kingdom

This branch of the todos sample app: https://github.com/blockstack/blockstack-todos/tree/feature/blockstack-19

It’s also used in the shipping version of blockstack-android https://github.com/blockstack/blockstack-android

We hope to ship this as blockstack.js 19 before the end of November.

2 Likes

This is very interesting, I think that in the future I’ll refactor Kit to use this to get User data, will probably make everything faster!

This seems like a good opportunity for the npm package to include both the existing webpacked browser dist file as well as the regular non-processed source that can provide nice code-completion / intellisense / docs in editors like VSCode. It also allows developers to specify their own packing for nodejs or web apps.

Attempting to import * as blockstack from 'blockstack/lib/index' doesn’t help since the ES6 modules in blockstack/src/*.js are destroyed when compiled to the blockstack/lib dir.

Documentation pages is great, but intellisense while coding against libs is much more convenient.

One approach would be performing the Flow transpile process without babel, then use babel/webpack to create the dist file. This wouldn’t change the existing workflow for those directly importing the dist for browser usage, and would allow importing from blockstack/lib/index for intellisense. I used a similar setup when playing around with porting blockstack.js from Flow to Typescript and it seemed to work well.

This would also fix another minor issue with the current setup where all (?) the package.json/dependencies should really be package.json/devDependencies.

This should probably be posted as a separate issue - just thinking if there have to be two breaking changes might as well be in the same release. Thoughts?

Example of VSCode unable to resolve anything useful from blockstack lib while coding:

1 Like

Good to know. Is there a migration path we’re recommending, or do we still need to write one?

I think this is a really great change, and brings about lots of benefits that have been discussed here.

I’m not super happy about making such a large breaking change, though. We already have issues where many apps aren’t using the most up-to-date version of blockstack.js, which can lead to issues during onboarding and elsewhere. In those cases, upgrading blockstack.js doesn’t even involve breaking changes.

With this change, apps will have to refactor their entire app. If those apps didn’t upgrade before, they surely won’t now, and existing apps will have to set aside some dev work to upgrade.

I don’t see why we can’t keep this as a non-breaking upgrade. We could still export all of the existing top-level metrics and have them use a UserSession object underneath. Then, all existing code would still work.

It could be something like this:

// blockstack.js top-level `getFile`
const getFile = (*args) => {
  const session = new UserSession();
  return session.getFile(*args);
}

I’d really not like to break existing web apps, because the vast majority of blockstack apps are web apps, and thus have no use for this new change. It seems like we’re making a big breaking change, for a very small amount of existing developers.

4 Likes