Create api modules for each core system

[mzsilak]

Currently all shared DTOs exist in the core-common project. With time it grows to a considerable size.
I would rather create a module for each core system which includes its models/DTOs and an interface for each service the system exposes.
The interface itself could be enriched with annotations to allow proxying to a transport.

This moves the effort to use a core service from each client to the service provider. Once the interface is written and annotated, all clients can use it.

See following example where I reused the bind parameters of spring controllers. (I guess our own annotations would be better here):

public interface TestInterface {

    @ArrowheadService(serviceDef = "A-METHOD", method = HttpMethod.POST)
    void aMethod(@RequestBody final Object parameter);

    @ArrowheadService(serviceDef = "SOME-SERVICE", method = HttpMethod.GET)
    @ResponseBody Object anotherMethod(@PathVariable("parameter") final Object parameter, @RequestParam("moreParameter") final Object moreParameters);
}

This would be the invocation handler which parses all annotations, makes a lookup in context/orchestration/serviceregistry and finally makes the query. If we are smart enough we could design protocol independent annotations be ready for future transports.

public class HttpInvocationHandler implements InvocationHandler {

    private final HttpService httpService;
    private final DriverUtilities driver;

    public HttpInvocationHandler(final HttpService httpService, final DriverUtilities driver) { super();
        this.httpService = httpService;
        this.driver = driver;
    }

    @Override
    public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable {
        final ArrowheadService service = method.getAnnotation(ArrowheadService.class);
        final UriComponents uriComponents = driver.findUri(service.serviceDef());
        final Object returnValue;

        switch(service.method()) {
            case POST:
                final Object payload = findRequestBodyParameter(method, args);
                // checking of other parameters
                returnValue = httpService.sendRequest(uriComponents, HttpMethod.POST, method.getReturnType(), payload);
                break;
            // some more cases ...
            default: throw new IllegalStateException("Unexpected value: " + annotation.method());
        }

        return returnValue;
    }
}

Creating and using the actual proxy is quite easy.

public class TestProxyFactory {

    private final HttpInvocationHandler invocationHandler;
    public TestProxyFactory(final HttpInvocationHandler invocationHandler) {this.invocationHandler = invocationHandler;}

    public <T> T createHttpProxy(final Class<T> cls) {
        return (T) Proxy.newProxyInstance(ClassLoader.getSystemClassLoader(), new Class[]{cls}, invocationHandler);
    }
}

Somewhere else in the code...

final TestProxyFactory proxy = new TestProxyFactory(httpInvocationHandler);
final TestInterface httpProxy = proxy.createHttpProxy(TestInterface.class);
httpProxy.aMethod("parameter");

What do you think?

[borditamas]

We have discussed this idea within AITIA.

The pacakage eu.arrowhead.common.dto.shared contains (and should contain) only those models/DTOs which are necessary for the application systems to interact with the AH Core or Support Systems. It shouldn't grow too big, but of course optimalization is always a "worth to talk" topic, especially in case of the clients.

But here are some concerns for further discussion:

(1) The current client-library was made with the intention of being as user friendly as possible. Bringing a ProxyFactory into, would make things more complicated and less understandable.
As an example I could only see that aMethod requires an Object input. I have to go through some documentations to know what kind of object is required in real. It's easy to spoil and not quite handy.

(2) The current client-library offers such methods which are dealing with more core-service in one call, which are also quite handy for the users. But if you can't be sure that all the necessary interface are implemented, then you can't provide such useful methods.

(3) With this kind of InvocationHandler the possible error response probably would be hidden somwhere under some kind of InvocationException, which again make things more complicated.

(4) We don't see how an HttpInvocationHandler could handle other protocols than http.

(5) If we are right InvocationHandler is using reflection to initiate the method in runtime, which is quite expensive. Quote from javadoc:

"Because reflection involves types that are dynamically resolved, certain Java virtual machine optimizations can not be performed. Consequently, reflective operations have slower performance than their non-reflective counterparts, and should be avoided in sections of code which are called frequently in performance-sensitive applications."

(+1) Tomcat isn't loading the application classes with SystemClassLoader to allow more Web-Application runing in different containers within the server. So the example code probably need to be corrected here.

[mzsilak]

Hello @borditamas,

Sorry, I could have made the example more clear.

(1) As an example I could only see that aMethod requires an Object input.

You can put in any concrete Type. All rules regarding java interfaces apply. Its super handy!

(2) But if you can't be sure that all the necessary interface are implemented, then you can't provide such useful methods.

I am not sure if I understand you correctly. The API interfaces I have in mind would be created by the developers of each core system. They surely know if a method is implemented or not.

(3) With this kind of InvocationHandler the possible error response probably would be hidden somewhere under some kind of InvocationException, which again make things more complicated.

Again, all rules of a java interface apply. If the method has a checked exception, you can throw that exception from the InvocationHandler. AFAIK, the same applies to RuntimeExceptions. Only Non-Runtime Exception are converted to InvocationException.

(4) We don't see how an HttpInvocationHandler could handle other protocols than http.

Well, it doesn't. You create a CoapInvocationHandler, a MqttInvocationHandler, ... whatever you need. The TestProxyFactory could be responsible for selecting the correct invocation handler for your needs. It could be also encoded into the interface to show that only specific transports are supported.

(5) If we are right InvocationHandler is using reflection to initiate the method in runtime, which is quite expensive.

We are using Spring IoC and Spring Data. We are using proxies all the time already. For example all classes which have @Transactional in one of the methods. All our @RestController's are proxies. The database repositories as well. .. ..

I found a very old article from Spring themselves on this topic: https://spring.io/blog/2007/07/19/debunking-myths-proxies-impact-performance/

[borditamas]

Hello,

(1) You can put in any concrete Type. All rules regarding java interfaces apply. Its super handy!

If you mean the way that you've used Object type in this example code only, and the real implementation would be with concrete type like "SystemRequestDTO" then it's OK. But if you mean that in the real implementation the methods could be called with any type of input, then we think it's problematic.

(2) I am not sure if I understand you correctly. The API interfaces I have in mind would be created by the developers of each core system. They surely know if a method is implemented or not.

Also not sure we are on the same understanding :). So we think, that you suggest to have each core/support system an own module which contains only it's own DTO classes, an interface class with the service methods, one (or more in the future) InvocationHandler class and a ProxyFactory class. This kind of package would be also published (one per core/support system) and client developers should decide which one they need to install for their application system. And this is the point where we see some disadventage.

• Without the manadatory core systems AH framework doesn't work, so it would more user friendly to provide at least these in one package.
• In the suggested way (if we correctly got it) you would provide only the pure service interfaces and the client developers should implement such frequent methods which are dealing with different core/support systems at once.
  In AH project we expect and also experience that not only professionals are developing java clients but pepole from quite various domains. So such a multifunctional package are very welcomed. Not saying that it wouldn't be possible with your solution, but it would require an additional package on the top which has all the "per system" package as transitive dependencies.

(3) If the method has a checked exception, you can throw that exception from the InvocationHandler.

You are right.

(4) Well, it doesn't. You create a CoapInvocationHandler, a MqttInvocationHandler, ... whatever you need

Understand.

(5) We are using Spring IoC and Spring Data. We are using proxies all the time already.

That's true, but we're using them mostly on server side and (accoring to our understanding) your suggestion would have effect on client side. The topic of "what counts as significant performance overhead" is really depending on the actual case. In AH community you can meet again with various opinions and requirements.
So our standpoint that it's true we are already using reflections even in client side but why we should add more if the same benefits are reachable without. I mean you suggest to hide the http request behind an interface with more reflection, but currently http requests are hidden behind to simple methods without additional reflections.

Don't know how much time you have, but if you mind youd could do a real implementation of your idea in DeviceRegistry. That way we could discuss all aspects really in detailed.

[mzsilak]

Hello,

(1) You can put in any concrete Type. All rules regarding java interfaces apply. Its super handy!

It would be like this (as mentioned with similar annotations):

public interface ServiceRegistry {
    @ArrowheadService(serviceDef = "service-query", method = HttpMethod.POST)
    public @ResponseBody ServiceQueryResultDTO queryRegistry(@RequestBody final ServiceQueryFormDTO form);

    @ArrowheadService(serviceDef = "service-register", method = HttpMethod.POST)
    public @ResponseBody ServiceRegistryResponseDTO registerService(@RequestBody final ServiceRegistryRequestDTO request);

    @ArrowheadService(serviceDef = "service-unregister", method = HttpMethod.DELETE)
    public void unregisterService(
            @RequestParam("service_definition") final String serviceDefinition,
            @RequestParam("system_name") final String providerName,
            @RequestParam("address") final String providerAddress,
            @RequestParam("port") final int providerPort);
}

The serviceDef value would be used for the lookup of the service if it is needed.

(2) api-modules
So we think, that you suggest to have each core/support system an own module which contains only it's own DTO classes, an interface class with the service methods ...

Yes.
The ProxyFactory and InvocationHandler class can come from a "common-api" module. There is no need to implement them over and over again. The client developers decide which one of the api modules they need based on which core systems they need to speak to.

Without the manadatory core systems AH framework doesn't work, so it would more user friendly to provide at least these in one package.

I don't disagree on that. They could be included in the "common-api" since that would be useful anyway.

In the suggested way (if we correctly got it) you would provide only the pure service interfaces and the client developers should implement such frequent methods which are dealing with different core/support systems at once.

No, the client developers do not need to implement the interfaces. That is the point. Just like we don't need to implement the interfaces of our SQL Repositories. The implementation is done dynamically through the InvocationHandler. IMHO, the implementation should deal with the lookup and the query. Also the InvocationHandler is only written once and it would be able to deal with all interfaces as long as the mandatory annotations are provided.

Not saying that it wouldn't be possible with your solution, but it would require an additional package on the top which has all the "per system" package as transitive dependencies.

I am not sure what you mean. In the Onboarding-Controller for example I need the AuthorizationService, OrchestrationService and CertificateAuthority. Having the mandatory core systems in a "common-api" module, I would only need to declare "certificate-authority-api" as my dependency and the "common-api" pulls it transitively.

(5) We are using Spring IoC and Spring Data. We are using proxies all the time already.

In the end, clients can choose which approach they would like to choose. I am not saying we should get rid of the existing HttpService class. Clients may still use that or implement their own solution.

I have to admit my main focus is support systems which run on sufficient hardware and to maximize re-usability. If you all dislike the idea of proxies, we could also make concrete classes for the api-modules, just like I did for the onboarding process in the eu.arrowhead.common.drivers packages (pending merge #171 )

I can probably implement an example which uses POST and reuses the existing HttpService.

kind regards,
Mario

[borditamas]

Hello,

I have to admit my main focus is support systems

Okay now it's much more clear and this explains why we are not on the same understanding regaring to point (2). We absolutely understand that "... the client developers do not need to implement the interfaces." What we are trying to pointed out is on top of that.

Due to "shared DTOs" (-> eu.arrowhead.common.dto.shared) was referred in your explanation we thought that this is all about to reforming the client-library, which is a more sensitive topic. Models/DTOs which are only necesarray for the communication between the core/support systems should go into the eu.arrowhead.common.dto.internal package. Currently every class placed in eu.arrowhead.common.dto.shared are going to automaticly built into an additional jar file for the client-library, so what is not necesarray for them those shouldn't be placed here.

Anyway, now it's clear that your conception would be managable without any impact on the client library.

I think that we should keep on discuss about aspects like:

• Should we use more reflection if similar benefits are achievable without? (and of course not with a ton of work like get rid of @Transactional just beacuse it's using reflection 😆 )
• Are the efforts of requierd refactorings (all the related implementations with all the unit tests we already have) in balance with the advantages of this solution.

[mzsilak]

Hello,

Should we use more reflection if similar benefits are achievable without? (and of course not with a ton of work like get rid of @transactional just beacuse it's using reflection 😆 )

Yes, we should use almost any means to make development faster for us and our clients. In case there is a real need for performance, we can troubleshoot the bottlenecks and tweak wherever its needed.
Very simplified, but JDK dynamic proxies use a special kind of reflection. Just like lambdas use a special kind of anonymous class. Both have a widespread use in the java ecosystem. I am curious on how well they perform on modern JVMs.

Are the efforts of requierd refactorings (all the related implementations with all the unit tests we already have) in balance with the advantages of this solution.

That is a good question.
Even if we create api-modules, either as Drivers or using Proxies, we should keep e.g. the HttpService because there are plenty of possible use cases for it.

Don't know how much time you have, but if you mind youd could do a real implementation of your idea in DeviceRegistry. That way we could discuss all aspects really in detailed.

I implemented an example of JDK Proxies under https://github.com/mzsilak/core-java-spring/tree/common-api. I think the implemented HttpInvocationHandler supports already a majority of existing systems, but testing is needed. It is written for HTTP(s) with JSON. As a side note, I used WebClient in this implementation, but RestTemplate would have worked as well.

The creation of the common-api module (moving classes from dto.shared) showed me that there are more dependencies than I was aware of. I think I also found a few cases where an internal dto was referenced from a shared dto and Utility classes.

In this prototype implementation I omitted the mentioned ProxyFactory. I think some kind of Builder is better suited if we want to support more kinds of InvocationHandlers in the future.
I filled in almost every annotation in the DeviceRegistryApi to show that they exist, however a lot of them actually have default values.

The deviceregistry-api has a main method in the test folder. It requires the core services running on localhost and presents 3 different errors / results.

Due to "shared DTOs" (-> eu.arrowhead.common.dto.shared) was referred in your explanation we thought that this is all about to reforming the client-library, which is a more sensitive topic.

I understand. I can imagine that the common-api is enough as dependency and that other api-modules are added as the client library evolves.

[mzsilak]

Hello @borditamas,

I experimented with JMH benchmarks and compared HttpService, Driver and the Proxy implementations. In the benchmark, each of those will do a ping to event handler and then publish an event. I have committed the benchmark and its result to my branch.

Following the results:

Implementation	1 Thread	4 Threads	20 Threads	Unit
Driver	232,482	444,154	476,433	ops/s
HttpService	227,317	431,599	480,405	ops/s
Proxy	237,092	751,857	1451,529	ops/s

JMH and Java versions:

JMH version: 1.23
VM version: JDK 11.0.4, Java HotSpot(TM) 64-Bit Server VM, 11.0.4+10-LTS
VM invoker: C:\Program Files\Java\jdk-11.0.4\bin\java.exe
VM options: -server
Warmup: 10 iterations, 10 s each
Measurement: 5 iterations, 30 s each
Timeout: 10 min per iteration
Benchmark mode: Throughput, ops/time

The results for Driver and HttpService are very similar which is not surprising because Driver uses HttpService as its backend. The Proxy also had similar performance values on single threaded execution and scales well with more threads. I would guess it is because of the React based WebClient. I am not sure if it really matters, but if it does, we can replace the RestTemplate with WebClient inside the HttpService.

I did not profile any of the implementations, but just created the benchmark tests and run them.

[borditamas]

Hello,

We should use almost any means to make development faster for us and our clients.

100% agree. After all, having an interface module per systems, we see as a good idea for the development of supporting systems.
Additionally to this we should take into consideration that only client endpoints will need to support other protocols than HTTP in the future. The internal communication between Core/Support systems should remain only HTTP based in order to keep it simple and easier to maintain.
According to our opinion the api modules as a driver kind of implementation would be the optimal solution, where

• the internal core system services would support only http,
• the public core system services would support http plus any other necessary protocol,
• the management endpoints wouldn't be covered at all.

we can replace the RestTemplate with WebClient inside the HttpService.

Yes we should do that, and lot of thanks for the benchmark.

[jerkerdelsing]

Let's lift this in the Roadmap session in the Vaccine WS, Wednesday April 14.