wisdom-framework / wisdom

A modular and dynamic web framework
http://wisdom-framework.org
Apache License 2.0
88 stars 42 forks source link

VertXSingleton doesn't use application.conf parameters #540

Closed nicolas-rempulski closed 8 years ago

nicolas-rempulski commented 8 years ago

Due to my history with Configuration services I'll be extra cautious on this one :

I try to mess with VertX ClusterWideMap and Hazelcast ReplicatedMap. When I tried to cluster wisdom, I found strange behaviour where VertX was unable to acquire ClusterWideMap.

Wisdom does not start in cluster mode even with vertx.clustered, verts.host-port, ... set in application.conf.

While tracing the behavior, it seems that Configuration in VertXSingleton is always null, even with parameters set in application.conf.

Shoudn't https://github.com/wisdom-framework/wisdom/blob/e73a3bade0b834f30a327127e9572140dff37fdd/core/wisdom-vertx-engine/src/main/java/org/wisdom/framework/vertx/VertxSingleton.java#L48 be mandatory and not nullable ?

cescoffier commented 8 years ago

Can you post your configuration.

nicolas-rempulski commented 8 years ago

As a reproducer, I used the wisdom:create goal (0.10.0-SNAPSHOT) and added the following lines to the default configuration :

vertx {
  clustered = true
  cluster-host = "localhost" # Address or IP used for the clustering
  cluster-port = 25501 # The port used for the clustering
}
cescoffier commented 8 years ago

Yes, it’s definitely a bug, the configuration is not bound at that time (well, may not be bound). Adding a @Requires on ApplicationConfiguration would fix it. WDYT ?

On 16 novembre 2015 at 14:35:33, Nicolas Rempulski (notifications@github.com) wrote:

As a reproducer, I used the wisdom:create goal (0.10.0-SNAPSHOT) and added the following lines to the default configuration :

vertx { clustered = true cluster-host = "localhost" # Address or IP used for the clustering cluster-port = 25501 # The port used for the clustering }

— Reply to this email directly or view it on GitHub.

nicolas-rempulski commented 8 years ago

I fixed it locally by replacing Configuration with ApplicationConfiguration and retrieving only the needed configuration in the @Validate

    @Requires(proxy = false)
    ApplicationConfiguration appConfiguration;
[...] 
    public void start() {
        Configuration configuration = appConfiguration.getConfiguration("vertx");

Downside is it brokes the tests and, admittedly, I'm quite lazy with tests! The moked configuration is no more valide (ApplicationConfiguration need to be moked)

singleton.configuration = new FakeConfiguration(ImmutableMap.of("clustered", false));