linagora / james-project

Mirror of Apache James Project
Apache License 2.0
70 stars 63 forks source link

[Postgres] Bind DistributedTaskSerializationModule into postgres-app #5150

Closed quantranhong1999 closed 3 months ago

quantranhong1999 commented 3 months ago

Why

The initial effort failed in https://github.com/apache/james-project/pull/2166.

1) [Guice/CanNotProxyClass]: Tried proxying JsonTaskSerializer to support a circular dependency, but it is not an interface.
  at DistributedTaskSerializationModule.taskCancelRequestedSerialization(DistributedTaskSerializationModule.java:71)
      \_ for 1st parameter

We still need to bind DistributedTaskSerializationModule to make the Distributed task manager fully work.

In the past, we had the same issue cf https://github.com/apache/james-project/pull/1208#issuecomment-1264375189, but somehow we do not have it anymore with the distributed-app.

I tried to figure out the way but failed.

How

Maybe now we have good reason to refactor a bit.

Should open 2 PRs against the postgresql and master branches to avoid conflict later.

Alternative

The same magic way we did with the distributed-app if any.

vttranlina commented 3 months ago

Should we upgrade to Guice 7.0.0 https://github.com/google/guice After upgrading to java21, I see a lot of code changes javax to jakarta

https://github.com/google/guice/wiki/Guice700

Guice 7.0 ONLY supports the jakarta.inject, jakarta.servlet and jakarta.persistence namespaces. It DOES NOT support javax.inject, javax.servlet, nor javax.persistence. The following links talk more about the jakarta transition: [Jarkata EE blog post](https://jakarta.ee/blogs/javax-jakartaee-namespace-ecosystem-progress/), [Eclipse Foundation blog post](https://eclipse-foundation.blog/2019/05/03/jakarta-ee-java-trademarks/), [Jakarta EE Wikipedia article](https://en.wikipedia.org/wiki/Jakarta_EE), [Oracle blog post](https://blogs.oracle.com/javamagazine/post/transition-from-java-ee-to-jakarta-ee)

[Guice 6.0](https://github.com/google/guice/wiki/Guice600) is being released alongside Guice 7.0 and is intended to help users migrate their code to the jakarta namespace. Guice 6.0 continues to fully support the javax.inject namespace while also mostly supporting the jakarta.inject namespace. The only part of Guice 6.0 that doesn't support jakarta.inject are the bind(..).toProvider methods.

The Guice 6.0 servlet & persist extensions only support the javax.servlet and javax.persistence namespaces respectively. If compatibility with jakarta.servlet or jakarta.persistence is required, use Guice 7.0.

Guice 7.0 removes support for javax.inject, javax.servlet and javax.persistence. Other than the namespace changes, Guice 6.0 & Guice 7.0 are identical.
quantranhong1999 commented 3 months ago

I am not sure it is related but I think it is good to upgrade Guice.

Arsnael commented 3 months ago

I agree, likely unrelated, but would be nice to upgrade it still :) please don't hesitate

vttranlina commented 3 months ago

pr fix

1) [Guice/CanNotProxyClass]: Tried proxying JsonTaskSerializer to support a circular dependency, but it is not an interface.

https://github.com/apache/james-project/pull/2172

Guice magical:

    private static final Function<PostgresJamesConfiguration, Module> POSTGRES_MODULE_AGGREGATE = configuration ->
        Modules.override(Modules.combine(
                new MailetProcessingModule(),
                new DKIMMailetModule(),
                POSTGRES_SERVER_MODULE,
                JMAP,
                PROTOCOLS,
                PLUGINS))
            .with(chooseEventBusModules(configuration));

it works