Fixed service loading of VerticleFactory from mvn exec java

[paulmiddelkoop]

Fixes #1437.

[vietj]

@cescoffier thoughts ?

[cescoffier]

@vietj Let me have a look once I'm back from my trip. I don't understand why we need explicit delegation in this case, and why the class cannot be loaded from the regular classpath (I also remember having fixed something quite similar in 3.2.1).

[paulmiddelkoop]

@cescoffier this is indeed similar to the fix you made in io.vertx.core.ServiceHelper. When dealing with different classloading mechanisms like OSGI and Maven plugin's, the current thread class loader will not work. I suspect that the Vert.x Service Factory will not be loaded correctly in the DeploymentManager when using OSGI in combination with the current thread class loader.

A similar problem is discussed here: http://stackoverflow.com/questions/36161795/how-to-register-a-spi-implementation-when-running-execjava.

[cescoffier]

Hi,

Do you have a quick reproducer of the issue. I tried with:

<execution>
            <id>run</id>
            <goals>
              <goal>java</goal>
            </goals>
            <configuration>
              <mainClass>io.vertx.core.Launcher</mainClass>
              <arguments>
                <argument>run</argument>
                <argument>${verticle.main}</argument>
                <argument>-cp</argument>
                <argument>${project.build.outputDirectory}</argument>
              </arguments>
            </configuration>
          </execution>

and it works for me (service loaded correctly) for a service defined in the project and a service loaded from a dependency.

That's said, I do agree with your fix, something is wrong here. However, I think it would be better to rely to the ServiceHelper.loadFactory method instead of passing a classloader, just to centralize all (META-INF) service loading.

[paulmiddelkoop]

Hi @cescoffier,

I added a reproduction project here: https://github.com/paulmiddelkoop/vertx-issue-1438.
When doing a mvn clean install exec:java it will throw an IllegalStateException since service descriptor is discarded. When running the Launcher directly or changing to the vertx version with my patch included it will print Loaded succesfull to the console.

I think ServiceHelper will be a good place to centralize service loading. At the moment this helper only returns the first factory. In the DeploymentManager case it should return all factories.

[cescoffier]

Thanks, for the reproducer, gonna have a look to it tomorrow.

[vietj]

@cescoffier I'm assuming we should close this one, right ?

[paulmiddelkoop]

I tested @cescoffier's fix and it works correctly, so this one can be closed.

[vietj]

great

[paulmiddelkoop]

Thanks @cescoffier !