james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matthieu Baechler <matth...@apache.org>
Subject About ConfigurationPerformer in James
Date Mon, 03 Jun 2019 14:45:31 GMT
Hi there,

I read this PR this afternoon 
https://github.com/linagora/james-project/pull/2398 and I found a
recurring problem here: 
https://github.com/linagora/james-project/pull/2398/files#diff-34220ca9b8dffd3a398bb2b3ce17c81eR108

As it's a problem that occurs frequently and pass reviews easily, I
think that it deserves an explanation about what is the expected design
because doing such errors probably proves that the explanations are
lacking.

Let's first explore the problem.

Problem 1.

Years ago, James was using Spring as the only way to do dependency
injection.

And it provides a nice feature call @PostConstruct. This annotation
allows to start your application it two phases: the first is injection,
the second is components initialization.

Guice doesn't offer that feature out of the box. So we tried to
implement that feature we some smarts use of Guice features (and as
everybody knows, trying to be smart is very often a very good way to
make things too hard understand).

To implement this @PostConstruct substitute, we created some classes
extending ConfigurationPerformer that would just call whatever methods
are useful to initialize components.

These ConfigurationPerformer classes are then added to a Guice
multibinder and as a last step, in server start method, we call these
ConfigurationPerformers.

It worked for some time and then it failed.

Problem 2.

Initialization is supposed to respect dependencies ordering. So calling
ConfigurationPerformers can fail because an object of type A is calling
an object of type B before B is initiliazed.

The solution is to listen to type injection to build a List (a List
keep insertion ordering) of the types that are injected in the order
they are created. As the injection framework creates objects with
respect to the dependency hierarchy, we then end up with the sequence
we need to respect for the initialization phase.

But this only give us the List of objects to initialize and not the
objects that perform the init. So we iterate the List and for each
element we try to find the matching ConfigurationPerformers. For this
to work, every ConfigurationPerformer has to declare what are the
classes it initializes with the famous forClasses method. For each
ConfigurationPerformer, we finally call the init method.

For the specific situation linked at the top of this email, the
ConfigurationPerformer declares MailboxIndexCreator as the type it
initializes. But MailboxIndexCreator is not injected anywhere, so it
will be created without any specific ordering.

The truth is: we want indices creation to happen before we start
objects that depend on an ElasticSearch connection, otherwise, the
indices may not exist in time for this object to work. So in this case,
the solution is quite simple: we probably have to declare
RestHighLevelClient or something related in the forClass of the
ConfigurationPerformer.

One last point that we have to look at is the interaction with
StartupCheck: do we want to try to create indices before checking
ElasticSearch version? (I guess not).

Thanks for reading and don't hesitate to ask if you need some more
explanations.

--
Matthieu Baechler


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message