Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ProjectServiceClient refactoring #4221

Closed
vzhukovs opened this issue Feb 24, 2017 · 7 comments
Closed

ProjectServiceClient refactoring #4221

vzhukovs opened this issue Feb 24, 2017 · 7 comments
Assignees

Comments

@vzhukovs
Copy link
Contributor

vzhukovs commented Feb 24, 2017

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 (#3248) and we can perform some refactoring tasks:

Needed for: #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.

@TylerJewell
Copy link

@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.

@gazarenkov gazarenkov removed the kind/epic A long-lived, PM-driven feature request. Must include a checklist of items that must be completed. label Jul 11, 2017
@gazarenkov
Copy link
Contributor

@vparfonov @dkuleshov could you please review

@dkulieshov
Copy link

dkulieshov commented Jul 11, 2017

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.

@gazarenkov
Copy link
Contributor

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

@dkulieshov
Copy link

dkulieshov commented Jul 11, 2017

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

@vparfonov
Copy link
Contributor

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

@ashumilova
Copy link
Contributor

not actual

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants