Che: ProjectServiceClient refactoring

Created on 24 Feb 2017  路  7Comments  路  Source: eclipse/che

PoC

There is a requirement to refactor existed org.eclipse.che.ide.api.project.ProjectServiceClient.

Due to upgrade a GWT library to 2.8, we can use java 8 features. So, there is no need to work with _Promises_ library and according to refactoring resource management (https://github.com/eclipse/che/issues/3248) and we can perform some refactoring tasks:

Needed for: https://github.com/eclipse/che/issues/4222

Changes

Refactor AsyncRequestCallback

There is a proposal to create an adapter for the org.eclipse.che.ide.rest.AsyncRequestCallback which can operate with java 8 consumers. _(issue link)_

Class diagram

requestcallbacktoconsumeradapter

Example of usage consumers in client service to obtain a DTO obejct:

    public void getItemMetaData(Consumer<DTO> success, Consumer<Throwable> fail) {
        reqFactory.createGetRequest(...)
                  .send(new RequestCallbackToConsumerAdapter<DTO>(unmarshallerFactory.newUnmarshaller(DTO.class)) {
                      @Override
                      public Consumer<DTO> getSuccessConsumer() {
                          return success;
                      }

                      @Override
                      public Consumer<Throwable> getErrorConsumer() {
                          return fail;
                      }
                  });
    }

Example of usage consumers in client service to obtain just success result (w/o DTO):

    public void getItemMetaData(Consumer<Void> success, Consumer<Throwable> fail) {
        reqFactory.createGetRequest(...)
                  .send(new RequestCallbackToConsumerAdapter<Void>() {
                      @Override
                      public Consumer<Void> getSuccessConsumer() {
                          return success;
                      }

                      @Override
                      public Consumer<Throwable> getErrorConsumer() {
                          return fail;
                      }
                  });
    }

Refactor AsyncRequest

Deprecations

  • Mark methods as deprecated _(issue link)_:

    • org.eclipse.che.ide.rest.AsyncRequest#send(org.eclipse.che.ide.rest.Unmarshallable<R>)

    • org.eclipse.che.ide.rest.AsyncRequest#send()

    • Remove deprecated methods in next sprints _(issue link)_

As far as these methods operates with _Promises_ they aren't efficient.

Refactor ProjectServiceClient

There is a proposal do decouple ProjectServiceClient component into two standalone components that will perform separate operations based on the physical objects (files, folders) and logical (projects).

Physical operations _(issue link)_

There is no need to mix file based operations with logical, so we can extract resource-based operations into component called FileSystemClient.

Class diagram

filesystemclient

Participants

  • FileSystemClient - main entry point for operating with resources' metadata;

    • injects FileSystemClientImplementorFactory and creates a new instance of FileSystemClientImp which holds in the class and not public for developers;
    • provides public consumer-based API for the operating with resources:
    • getItemMetaData;
    • createEmptyFile;
    • readFileContentAsText;
    • writeNewFileContent;
    • createFolder;
    • deleteItemMetaData;
    • copyItemMetaData;
    • moveItemMetaData;
    • getChildrenMetaData;
    • getTreeMetaData;
    • search;
    • delegates requests from the public API into FileSystemClientImp.
  • FileSystemClientImp - implementation bridge for the FileSystemClient that allows to be extended independently from the FileSystemClient, at the moment duplicates the public API.

  • FileSystemClientImplementorFactory - factory for producing implementation for the implementor bridge.

  • FileSystemApiModule - Gin module for configuring main components that works together on the ide-app part;

    • binds FileSystemClient as Singleton;
    • binds FileSystemClientImplementorFactory to WsAgentFileSystemImplementorFactory as Singleton.
  • WsAgentFileSystemImplementor - bridge implementation for the communicating with particular workspace agent, implements FileSystemClientImp.

  • WsAgentFileSystemImplementorFactory - factory for producing a new instance of WsAgentFileSystemImplementor.

Logical operations _(issue link)_

As far as file based operations moved to the standalone component, operations with projects also might be moved to component called ProjectClient

Class diagram

projectclient

Participants

  • ProjectClient - main entry point for operating with projects' metadata;

    • injects ProjectClientImplementorFactory and creates a new instance of ProjectClientImp which holds in the class and not public for developers;
    • provides public consumer-based API for the operating with project configurations:
    • getProjectsConfiguration;
    • getProjectConfiguration;
    • updateProjectConfiguration;
    • createProject;
    • createProjects;
    • importProject;
    • resolveFolder;
    • estimateFolder;
    • delegates requests from the public API into ProjectClientImp.
  • ProjectClientImp - implementation bridge for the ProjectClient that allows to be extended independently from the ProjectClient, at the moment duplicates the public API.

  • ProjectClientImplementorFactory - factory for producing implementation for the implementor bridge.

  • ProjectApiModule - existed Gin module, that adds additional configuration for main components that works together on the ide-app part;

    • binds ProjectClient as Singleton;
    • binds ProjectClientImplementorFactory to WsAgentProjectCLientImplementorFactory as Singleton.
  • WsAgentProjectClientImplementor - bridge implementation for the communicating with particular workspace agent, implements ProjectClientImp.

  • WsAgentProjectClientImplementorFactory - factory for producing a new instance of WsAgentProjectClientImplementor.

Most helpful comment

We don't need refactoring just for refactoring, if it not solve some concrete problem. At least for now

All 7 comments

@vzhukovskii @ashumilova - Is this still actual and if it is an epic, do we have a list of issues that we must tackle on this? Thanks.

@vparfonov @dkuleshov could you please review

Due to upgrade a GWT library to 2.8, we can use java 8 features.

I must admit that that's a very bright idea however at the moment we can't completely stop using javascript promise wrappers until we will have appropriate alternatives inside GWT (e.g. CompletableFuture). So here we should proceed with caution to leave backward compatibility.

Examples of usage of consumers in client service looks just fine to me, I would also recommend to overload single parameter methods like

public void getItemMetaData(Consumer<DTO> success) {
        reqFactory.createGetRequest(...)
                  .send(new RequestCallbackToConsumerAdapter<DTO>(unmarshallerFactory.newUnmarshaller(DTO.class)) {
                      @Override
                      public Consumer<DTO> getSuccessConsumer() {
                          return success;
                      }

                      @Override
                      public Consumer<Throwable> getErrorConsumer() {
                          return t -> {};
                      }
                  });
    }

or something similar for all possible variants.

There is a proposal do decouple ProjectServiceClient component into two standalone components that will perform separate operations based on the physical objects (files, folders) and logical (projects).

In general sounds quite reasonable. Speaking about the implementation itself I would argue for method names and purposes. However that's a topic for more specific implementation-related discussion.

In my opinion the author underlined an important task of moving our project for a deep support of java 8 features, which is obviously a pretty important initiative in general. Though the description lacks implementation details to have more accurate understanding of author's vision, the idea in general seems to be quite okay so I guess we can proceed with further more specific conversations in dedicated issues.

My only concern here is that we're in a process of moving some REST-based functionality to JSON-RPC based and that would obviously impact the project service, so it might be a good idea to finish the transition to the new architecture first. It is quite possible that moving to the new way of communication will change the ProjectServiceClient to some degree as well, so this specification may become outdated.

May I conclude that it's a good idea in general but not the best time to implement?

That is a brief vision of what I was trying to say :-)

We don't need refactoring just for refactoring, if it not solve some concrete problem. At least for now

not actual

Was this page helpful?
0 / 5 - 0 ratings