GMOD / Apollo

Genome annotation editor with a Java Server backend and a Javascript client that runs in a web browser as a JBrowse plugin.
http://genomearchitect.readthedocs.io/
Other
124 stars 85 forks source link

Ability to use multiple organisms at the same time causes issues #441

Closed cmdcolin closed 8 years ago

cmdcolin commented 9 years ago

I am not sure if there is an issue open for this already, but here is the scenario:

To reproduce, open an organism 1 in tab A Then open organism 2 in tab B Then go back to tab A and try and perform actions (turn on tracks, create annotation, etc)

The result is that anything that now happens in tab A is invalid. The tracks or annotations that you try to create on tab A are invalid because when you opened up tab B, it set that as the "active organism" and then anything in tab A is invalid. Furthermore, these actions in tab A don't really produce error messages which makes it confusing.

nathandunn commented 9 years ago

We should make this a more stateless app to some degree, but retaining preferences on the backend so when you come back, you'll be where you left off (right?). Will require some additional discussion.

nathandunn commented 8 years ago

This is a duplicate, which we've already fixed.

cmdcolin commented 8 years ago

I'm not sure this was fixed.

nathandunn commented 8 years ago

Alright . . I will assign it then.

cmdcolin commented 8 years ago

It would be great if the Apollo2 used "routes" in the URL, for instance

http://host/apollo/MY_ORGANISM/annotator

and

http://host/apollo/MY_ORGANISM/jbrowse

that would allow links to both the annotator panel in the former, and jbrowse only in the latter

It might also reduce the amount of logic that is currently trying to figure out what the current organism is from user preferences, etc.

cmdcolin commented 8 years ago

It might also be possible that we can just create new additions to UrlMapping to test this out and not affect the current code. Not sure yet.

cmdcolin commented 8 years ago

There is a definite case where this needs fixing too:

Currently we recommend that all users for the webapollo demo login to demo@demo.com

If two users are doing that and browsing different organisms, that will cause problems and their browsing and annotating will fail

nathandunn commented 8 years ago

Yes, this is true. However, this is a bad use-case. A true “production” instance should never have this problem.

It does make me think that allowing for auto-registration somehow to be supported on the demo side would not be a bad idea.

Nathan

On Sep 2, 2015, at 9:59 AM, Colin Diesh notifications@github.com wrote:

There is a definite case where this needs fixing too:

Currently we recommend that all users for webapollo login to demo@demo.com mailto:demo@demo.com If two users are doing that and browsing different organisms, that will cause problems and their browsing and annotating will fail

— Reply to this email directly or view it on GitHub https://github.com/GMOD/Apollo/issues/441#issuecomment-137170843.

cmdcolin commented 8 years ago

Agree.

nathandunn commented 8 years ago

We need a per-"window" preference. If window is new, it should select one of the existing preferences (and remove the older one).

monicacecilia commented 8 years ago

This is definitely a scenario we are expecting to see - one researcher looking at two different genomes to compare, in two windows, concurrently. Good catch @cmdcolin Thanks for looking into this @nathandunn

nathandunn commented 8 years ago

Session Storage seems to be the best way to do this:

http://stackoverflow.com/questions/368653/how-to-differ-sessions-in-browser-tabs

nathandunn commented 8 years ago
cmdcolin commented 8 years ago

I am confused by the approach here. I think really the best approach is to just use the URL to figure out what organism someone is browsing, it just makes it explicit.

This makes it simpler not just for the server but the client too! It let's you just copy and paste to URL to share, which I think is great

nathandunn commented 8 years ago

Yeah, having things come in on a relative URL would make sense. I still think you want want a preference, though.

Probably the easiest would be setting the preference in the URL, where it is either an organism or a client (individual tab) token:

http://localhost:8080/apollo//jbrowse/index.html?loc=GK000029.2%3A1..174775&highlight=&tracklist=1&clientToken=434450158&tracks=Annotations&nav=1&overview=1

I think this allows a great deal of flexibility. For a given token, we can do a lookup in the order of: 1 - client token (and any associated preferences for that tab, including user and sequence). This will set the preference for that token such that if have other services, they only need the token in the relative URL to know what directory they should use. In this case if there is a conflict with this URL and the token it sets preferences on the token (e.g., navigating between screens). 2 - organism ID: just the organism ID and looks up the last known preference for that and sets that sequence / location and recreates the token and does a refresh (probably). 3 - organism Name: same as Organism ID, but does the lookup

cmdcolin commented 8 years ago

I think the session storage is an interesting way to address this, but I think that having a "semantic URL" approach is even more versatile.

The URL approach allows links that even non technical users can understand, for example a simple URL that says like http://localhost/Apollo2/Amel_4.5/annotator

The URL format is really tangential, the thing that is important is that the organism is just resolved properly. If the organism is made explicit, this is easy. If we continue to rely on things like "current organism" then we can have continued problems, while client token is interesting, it could have problems too, and it also makes all Preferences ephemeral by tying it to the "session storage" (which is a HTML5 feature to store things only in the current tab)

nathandunn commented 8 years ago

I think that its important to note that the previous system was dependent on file-system paths and/or symlinks. Obviously we don’t want to expose this.

A client token is similar to a semantic URL . . or at least as close as we’re going to get. You should be able to use an organism as the prefix to the URL, but really the token for the browser is the more important thing. SessionStorage is the “root” storage specifically because it is tab-specific, though we can retain info across multiple sessions.

The reason for having a “currentOrganism” is that it allows a single client token to maintain references to multiple organisms so that if you go back and forth between two organisms you return to your prior location.

Nathan

On Apr 12, 2016, at 2:24 PM, Colin Diesh notifications@github.com wrote:

I think the session storage is an interesting way to address this, but I think that having a "semantic URL" approach is even more versatile.

The URL approach allows links that even non technical users can understand, for example a simple URL that says like http://localhost/Apollo2/Amel_4.5/annotator

The URL format is really tangential, the thing that is important is that the organism is just resolved properly. If the organism is made explicit, this is easy. If we continue to rely on things like "current organism" then we can have continued problems, while client token is interesting, it could have problems too, and it also makes all Preferences ephemeral by tying it to the "session storage" (which is a HTML5 feature to store things only in the current tab)

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/GMOD/Apollo/issues/441#issuecomment-209108995

nathandunn commented 8 years ago

@cmdcolin WRT to cleaning up security . . . I think that the grails security plugin does a better job of this than shiro. In that case you annotate by method / class. The gist of it, though, is that you are basically adding a custom beforeInterceptor in the annotation. You could create your annotation to do this in the same manner, so:

def someMethod(JSONObject inputOBject){
        Sequence sequence = permissionService.checkPermissions(inputObject, PermissionEnum.WRITE)

Would become something like:

@CustomApolloSecurity(PermissionEnum.WRITE)
def someMethod(JSONObject inputObject){

In this case CustomApolloSecurity would be a beforeInterceptor that could read the params passed into the method. Maybe something we can explore once we change the security plugin.

cmdcolin commented 8 years ago

This would be a great improvement :)

monicacecilia commented 8 years ago

💃