eclipse-emfcloud / emfcloud-modelserver

Modelserver component
Other
43 stars 21 forks source link

Consistently handling URIs with the Model Server #155

Closed CamilleLetavernier closed 2 years ago

CamilleLetavernier commented 2 years ago

This ticket is a bug, but also a discussion to make sure URIs are handled consistently on the Model Server. I'm especially focusing on the "ModelURI query parameter", used for the REST APIs (XMI URIs will be handled as usual by EMF).

In V1/Theia, the client used to know the Workspace URI, and could specify absolute file URIs. It seems that several cases were possible:

/path/to/ws-root/project/folder/file file:/path/to/ws-root/project/folder/file file:///path/to/ws-root/project/folder/file

But some URIs that aren't using the file:/ protocol were also mentioned, e.g. in issue #72:

After second though, I'm also concerned with what may happen to non-file URIs... like a platform:/resource/ or cdo:// URI (which is absolute).

Moving towards the ModelServer V2 API, the client is now standalone, and doesn't know about the Workspace/Workspace Root anymore. We typically expect relative URIs (or opaque URIs) instead, which the server provides via the /models endpoints:

project/folder/file

(Technically, this is treated as a URI relative to the workspace root, but it should really be considered an opaque ID from the client's point of view).

With the current state, there are some inconsistency across the different platforms (Especially Linux vs Windows, where the C:/ segment is sometimes omitted during the URI conversion. This causes e.g. eclipse-emfcloud/ecore-glsp/issues/112)

In the code, we have several occurrences of String-to-URI conversion. For example:

org.eclipse.emfcloud.modelserver.emf.common.DefaultUriHelper.toFileUri(String)

   public Optional<URI> toFileUri(final String fileUrl) {
      try {
         String decodedUrl = URLDecoder.decode(fileUrl, "UTF-8");
         File file = getAbsoluteFile(decodedUrl);
         URI uri = withTrailingSeparator(URI.createFileURI(file.toURI().normalize().getPath()));
         return Optional.ofNullable(uri).filter(URI::isFile);
      } catch (NullPointerException | IllegalArgumentException | UnsupportedEncodingException e) {
         LOG.warn(String.format("Could not convert to filePath! ’%s’ is not a valid URL", fileUrl));
         return Optional.empty();
      }
   }

And:

org.eclipse.emfcloud.modelserver.emf.common.DefaultModelResourceManager.adaptModelUri(String)

   public String adaptModelUri(final String modelUri) {
      URI uri = URI.createURI(modelUri, true);
      if (uri.isRelative()) {
         if (serverConfiguration.getWorkspaceRootURI().isFile()) {
            return uri.resolve(serverConfiguration.getWorkspaceRootURI()).toString();
         }
         return URI.createFileURI(modelUri).toString();
      }
      // Create file URI from path if modelUri is already absolute path (file:/ or full path file:///)
      // to ensure consistent usage of org.eclipse.emf.common.util.URI
      if (uri.hasDevice() && !Strings.isNullOrEmpty(uri.device())) {
         return URI.createFileURI(uri.device() + uri.path()).toString();
      }
      return URI.createFileURI(uri.path()).toString();
   }

I will provide a quick-fix for EcoreGLSP on V1/Windows (Simply making sure we don't lose the C: segment in that specific case), but it would be nice to formally clarify how we expect Model URI query parameters to be specified (and returned, when using the /models endpoint).

@vhemery mentioned use cases with CDO: can you clarify how you currently use the modelURI query parameter? Do you allow arbitrary URIs, like we currently do in the ModelServer? With an opaque URI format, Clients wouldn't be able to do that anymore; they would have to pick models from a list of known models returned by the model server (As currently done on the /models endpoint). Would that still work for you?

(Note: at the moment, on both V1 and V2, the /models endpoint still returns absolute file:/ URIs. On V2, we'll need to change that to workspace-relative URIs. The Java Client or GLSP-ModelServer integration might also need to be updated, if they try to handle these values as EMF URIs).

vhemery commented 2 years ago

@vhemery mentioned use cases with CDO: can you clarify how you currently use the modelURI query parameter? Do you allow arbitrary URIs, like we currently do in the ModelServer? With an opaque URI format, Clients wouldn't be able to do that anymore; they would have to pick models from a list of known models returned by the model server (As currently done on the /models endpoint). Would that still work for you?

(Note: at the moment, on both V1 and V2, the /models endpoint still returns absolute file:/ URIs. On V2, we'll need to change that to workspace-relative URIs. The Java Client or GLSP-ModelServer integration might also need to be updated, if they try to handle these values as EMF URIs).

I do accept arbitrary URIs, as long as they are understood by EMF and hence our model server implementation.

Work on the end-to-end CDO integration should be performed during 2022. It is not functional yet on our side. We plan to use cdo uris such as cdo://DISCO-RemoteRepository/MyProject/MyModel.disx

On the other hand, I do have some http:// URIs mapped with plugin resources on which I use the 'typeschema' endpoint. E.g. http://www.cnes.fr/disco/domain-schemas/structuredData/0 with

   <extension point="org.eclipse.emf.ecore.uri_mapping">
      <mapping
            source="http://www.cnes.fr/disco/domain-schemas/structuredData/0"
            target="platform:/plugin/fr.cnes.disco.model.profiles.structureddata/model/definitions/0/structuredData.ecore">
      </mapping>
   </extension>

We also use absolute file uris.

I don't think we would be able to perform everything we want with only opaque non-hierarchical URIs and relative URIs. But we already have our own versions of ModelRepository and ModelResourceManager (and even ModelController). So if we are still able to override the behavior with these classes, it is not a problem at all.

For the moment, we do not use the getAll version of the 'models' endpoint. Because indeed, we do not know in advance where all our usable models may be (repositories, platform:/plugin, mapped uris, file:// ...).

I think that in worst case scenario, if we can still use opaque URIs such as scheme:example.com/path, we would just map opaque URIs to the hierarchical URIs we are interested in... Either with an opaque to hierarchical mapping logic, or with an extra service which would first register the hierarchical URI and give back the fixed opaque URI to use instead.

I suggest you make the required changes to sabilize things with the best compatibility effort. Then we will worry of whether it really break things for DISCO, and how we can fix it (either directly on our side or by contributing to the core). Do not hesitate asking me for a review if you feel a particular commit may be breaking compatibility.