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

Extract DI to separate module #711

Closed
wants to merge 74 commits into from

Conversation

eutkin
Copy link

@eutkin eutkin commented Mar 11, 2020

I've made life cycle management of scalecube services a separate subsystem: ServicesProvider.

Pro:

  • The service lifecycle management logic is now concentrated in one class (ServicesProvider);
  • You can replace the implementation with another IoC container (Spring/Guice/etc).
  • The API is backwards compatible

idea

ServicesProvider - interface for IoC container that performs several functions:

  • stores all instances of services;
  • manages the life cycle of stored services (creation -> initialization -> finalization);
  • gives meta information about stored services (type and tags).

Scale Cube for DI proprietary mechanisms are collected in one class, in ServicesProvider implementation (see ScaleCubeServicesProvider).

The Spring IoC implementation is also given for the example.

@eutkin eutkin linked an issue Mar 11, 2020 that may be closed by this pull request
@eutkin eutkin changed the title Extract DI to separate module WIP: Extract DI to separate module Mar 11, 2020
@eutkin eutkin requested a review from artem-v March 12, 2020 11:25
Eugene_Utkin added 2 commits June 13, 2020 17:31
…e MicroservicesContext from ServiceFactory#getServiceDefinitions
@eutkin eutkin requested a review from artem-v June 13, 2020 15:21
@eutkin eutkin requested a review from artem-v June 13, 2020 17:14
@FunctionalInterface
@Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eutkin @eutkin
Coming back to this question.

Again, why those methods were deprecated:

services(Object... services)
services(ServiceInfo... services)
services(ServiceProvider serviceProvider)

Can't u keep them and use ScalecubeServiceFactory which will be defined internally?

services(Object... services)
services(ServiceInfo... services)
services(ServiceProvider serviceProvider)
services(ServiceFactory serviceFactory)

I.e first three works with internal ScalecubeServiceFactory (client will not know even) + last one allows to specifiy explicitly serviceFactory. This way u would not need to deprecate you would just expose internal toolset called ServiceFactory.

ServiceInfo serviceInfoB = ServiceInfo.fromServiceInstance(serviceB).build();
return Mono.just(Collections.singletonList(serviceInfoB));
}
};
Copy link
Contributor

@artem-v artem-v Jun 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized how ServiceFactory api can shoot in the foot. In this example with service-a service-b in method getServiceDefinitions were returned two services, in method initializeServices was returned one service. That's ok that service-b injects service-a, but i imagine scenario with N services and now client has to maintain correlation bewtween what's returned from getServiceDefinitions and initializeServices.
Don't have solution for this, but to me it looks like a big problem.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are only two ways.

  1. We create instances of services from ServiceDefinition

It is assumed that ServiceFactory will delegate its task to the DI framework and there will be no troubles. But they are possible. I will think about it.

e.utkin added 11 commits June 13, 2020 23:08
…module

# Conflicts:
#	services/src/main/java/io/scalecube/services/Microservices.java
#	services/src/test/java/io/scalecube/services/sut/AnnotationServiceImpl.java
#	services/src/test/java/io/scalecube/services/transport/rsocket/RSocketNettyColocatedEventLoopGroupTest.java
@eutkin eutkin requested a review from artem-v June 17, 2020 08:40
@eutkin
Copy link
Author

eutkin commented Jun 18, 2020

up

e.utkin added 3 commits June 19, 2020 17:18
…module

# Conflicts:
#	services-examples-parent/services-examples/src/main/java/io/scalecube/services/examples/discovery/CompositeDiscoveryExample.java
#	services/src/main/java/io/scalecube/services/Microservices.java
@eutkin
Copy link
Author

eutkin commented Jun 22, 2020

What other important problems remain unresolved? It's becoming more and more difficult to merge this PR with the develop branch.

@eutkin
Copy link
Author

eutkin commented Jun 22, 2020

@ronenhamias @artem-v @segabriel

When I was writing the documentation, I found a semantic mistake. I suggest changing ServiceFactory in this way.

Why? Because otherwise it seems as if we are managing the life cycle of only those services that will be placed on the Scalecube node. In fact, ServiceFactory is needed to manage the lifecycle of all user components. But unlike DI frameworks ServiceFactory separates Scalecube services from all components.

@segabriel
Copy link
Member

@eutkin

What other important problems remain unresolved? It's becoming more and more difficult to merge this PR with the develop branch.

I guess we need to postpone this PR for a week probably two weeks, due to we need to have a stable version of scalecube-services for this time slot. And have the ability to fast solve some shortcomings that stop us.

Base automatically changed from develop to master September 17, 2020 21:36
@artem-v artem-v closed this May 19, 2023
@artem-v artem-v deleted the feature/issue-708-di-security-extract-to-module branch May 19, 2023 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Separating non-core functionality into separate modules
4 participants