[layout restoration] Store per location

[svenefftinge]

Currently, we only store per host and workspace. As a result theia tries to restore the layout whenever those two match. When starting different workspaces in the same container those two informations are always the same although the content is not. To distinguish the location (URL) needs to be taken into account, too.

[hexa00]

Humm this will be a problem with an implementation I know in which the container port is dynamic but is the same container.

So when reconnecting to the same container after some time, the port will change and thus it will appear as the same workspace :(

Could we instead use some kind of token that the backend would provide?

[svenefftinge]

Had to revert the commit, due to #943

[marcdumais-work]

I noticed this recently: In an internal tool that serves Theia containers, a reverse proxy is used, that "hides" the port number being used, for a given container. Then the user connects to a specific instance using a path instead of a port number. e.g. :

https://server.com/user/proj/container
instead of
https://server.com:12345

In this situation, the "origin", as used to save the layout info into "Local Storage" is https://server.com, without a port number. So if a user has multiple containers on that server, and some have the same workspace path, then they will all share the same Local Storage "slot", overwriting each other.

@svenefftinge a slight modification to your reverted PR seems to work in this case, as well as on Electron: instead of adding window.location.toString() to the key, I added window.location.pathname. The later does not contain the dynamic port, on Electron, and so works as expected. Note that the resulting key looks a bit messy, on Electron.

[paul-marechal]

I think that just like @hexa00 said, using a token could allow us to identify a particular instance, without worrying much about path changes.

What about looking for some kind of instance.id inside Theia's installation folder and fetching the key from there ? If the file does not exist, then create it and put a random uuid in there. When you want to move things around, you can keep this file and put it in the new Theia installation folder.

Maybe make Theia look for this on backend boot up ?

[marcdumais-work]

@marechal-p this sounds like a nice way to do it. Then we would need to use that id as part of the key, when we save/restore the workbench layout in/from Local Storage.

[simark]

I think that the unique id should uniquely identify the workspace, not a Theia installation or instance. Opening two different workspaces with the same Theia instance or installation should result in two different local data sets. Opening the same workspace using two different Theia instances or installations should result in accessing the same local data set.

From that point of view, I think that when it starts, Theia should read that unique id from a file in the workspace (let's say .theia/id.json). If it's not present, it should generate a uuid and save it there. That uuid would be used as part of the key to access the local storage.

What I am not sure is whether the www path or the filesystem path of the workspace should go into that key or not.

If I move my workspace directory and open it again (at the new location), I'd like if we kept the same local data. However, if I copy my workspace and open it separately (so to have two working copies of the same code), they should be considered as two different workspaces. And since I copied everything, including .theia/id.json, it sounds like we want to consider the workspace path in that case to avoid collision between the originalk and the new workspace.

[marcdumais-work]

If I move my workspace directory and open it again (at the new location), I'd like if we kept the same local data

I think that ATM we save absolute paths of files, for example to be able to re-create open editors. If we want the above to work, we will need to save the workspace-relative path instead or interpret the paths according to the workpace.

[svenefftinge]

I think all this is highly depending on how Theia is used (i.e. how the workspace server works).
There are various ways a workspace can be identified : in URLs through domain names, pathes, ports, or through a custom file if you think that fits best with your architecture.
Therefore, I 'd like to keep the current simple implementation in Theia.
Could you tailor it outside Theia, i.e. in an extension specific to your way of hosting Theia?

[marcdumais-work]

@svenefftinge I tend to agree, that we can keep this as simple as possible. So we will consider doing an internal customization if we want to do something custom for our use case.

With the current master, I think we are very close to have a recipe that makes this work for all basic cases above. The "Path without port" case is still problematic. Do you see cases where adding the path to the key would break the mechanism?

e.g., based on your previous, reverted, PR (this version doesn't break vanilla Electron Theia, since the randomly generated port is not added to the key):

marcdumais-work@3d177c7

[svenefftinge]

I think excluding the port is actually also very specific to your workspace server.
A generic solution would be, to use the workspace path plus the location.href (maybe a short hash of the two, because the key is stored very often).

[paul-marechal]

Use case:

• People deploy a container running Theia, for a workspace W with project P
• Reverse proxy maps the container back to a user U to access his container.
• URI is something along: https://reverseproxy/U/W/P.
• In this case the local storage domain is reverseproxy, we will always end up accessing keys for this domain, no matter what, this because of the way the local storage seems to work (per host:port).

In this scenario, using location.href is fine because the projects seems to be identified by URI path.
Thing is, this prevents the user from renaming the project as Q for instance, and he looses the localstorage data that was defined for his project, because the URI path may become /U/W/Q instead of U/W/P. Using some kind a workspace identifier can help here.

We were talking about using an environment variable to override the URL path in case the URL is irrelevant (in case users use reverse proxying again, a valid use case for a service hosted in the cloud).

Basically:

if (envWorkspace) {
    prefix = `${envWorkspace}:${onDiskProjectPath}`;
} else {
    prefix = `${window.location.pathname}:${onDiskProjectPath}`;
}

Using window.location.pathname because electron adds a query string ?port=random which is stripped when calling pathname.

Would this be generic enough to support both localhost editing and more specific cloud services ?

edit: we could also use cli params instead of an environment variable, but this would allow us to identify remote instances behind the same proxy, based on the server's way of managing theia.

[svenefftinge]

Using some kind a workspace identifier can help here.

Yes, but that is the responsibility of the workspace server. At TypeFox, for instance, we use subdomains with a workspaceid. In your case it is pathes. We have an abstraction in place that can be implemented to your needs. There is no need to introduce the notion of a workspace id or 'domain'. Just solve it for your scenario and keep the generic solution a good but simple 80% case.

[paul-marechal]

Just in case I made a parallel branch with the change adding some kind of "identification" for the case where theia would be behind a proxy.

In this case, the proxy can move the container access point any way possible (because it could change the project name or container name or anything).
edit: by access point I mean URL

In this case the admins can just specify an --id=xxx so that despite any change in the route to access theia, the localstorage is capable of keeping track of it. If we don't specify it, it uses the window.location.pathname as fallback.

master...marechal-p:mp/issue-904b

But if you still feel like this is not a valid issue then I'm fine with the current open PR about this.

[akosyakov]

@svenefftinge @marechal-p Could it be closed?