Skip to content

Commit

Permalink
JAMES-2854 ConfigurationPerformer should be bound to a single startable
Browse files Browse the repository at this point in the history
Loops are supported by injection framework through stubbing. However, this
does not play well with the "initialization will follow injection path"
strategy: some modules might try to init against "not yet initialized" components
that were stubbed by the init strategy.

The issue here is that multi-components configuration performers adds additional complexity,
and creates dependencies (init A & B at the same time) that lead to failures
while modifying some injections.

Saying "1 startable = 1 performer" prevents us from this, and fits our usage.
  • Loading branch information
chibenwa committed Aug 20, 2019
1 parent 2137385 commit 7faac28
Show file tree
Hide file tree
Showing 31 changed files with 107 additions and 253 deletions.
Expand Up @@ -18,8 +18,6 @@
****************************************************************/
package org.apache.james.modules.data;

import java.util.List;

import org.apache.commons.configuration2.ex.ConfigurationException;
import org.apache.james.backends.cassandra.components.CassandraModule;
import org.apache.james.domainlist.api.DomainList;
Expand All @@ -29,7 +27,6 @@
import org.apache.james.server.core.configuration.ConfigurationProvider;
import org.apache.james.utils.ConfigurationPerformer;

import com.google.common.collect.ImmutableList;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Provides;
Expand Down Expand Up @@ -59,7 +56,6 @@ public DomainListConfiguration provideDomainListConfiguration(ConfigurationProvi

@Singleton
public static class CassandraDomainListConfigurationPerformer implements ConfigurationPerformer {

private final DomainListConfiguration configuration;
private final CassandraDomainList cassandraDomainList;

Expand All @@ -79,8 +75,8 @@ public void initModule() {
}

@Override
public List<Class<? extends Startable>> forClasses() {
return ImmutableList.of(CassandraDomainList.class);
public Class<? extends Startable> forClass() {
return CassandraDomainList.class;
}
}
}
Expand Up @@ -18,8 +18,6 @@
****************************************************************/
package org.apache.james.modules.data;

import java.util.List;

import org.apache.commons.configuration2.ex.ConfigurationException;
import org.apache.james.backends.cassandra.components.CassandraModule;
import org.apache.james.lifecycle.api.Startable;
Expand All @@ -31,7 +29,6 @@
import org.apache.james.server.core.configuration.ConfigurationProvider;
import org.apache.james.utils.ConfigurationPerformer;

import com.google.common.collect.ImmutableList;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Scopes;
Expand All @@ -53,7 +50,6 @@ public void configure() {

@Singleton
public static class CassandraRecipientRewriteTablePerformer implements ConfigurationPerformer {

private final ConfigurationProvider configurationProvider;
private final CassandraRecipientRewriteTable recipientRewriteTable;

Expand All @@ -73,8 +69,8 @@ public void initModule() {
}

@Override
public List<Class<? extends Startable>> forClasses() {
return ImmutableList.of(CassandraRecipientRewriteTable.class);
public Class<? extends Startable> forClass() {
return CassandraRecipientRewriteTable.class;
}
}

Expand Down
Expand Up @@ -18,8 +18,6 @@
****************************************************************/
package org.apache.james.modules.data;

import java.util.List;

import org.apache.commons.configuration2.ex.ConfigurationException;
import org.apache.james.backends.cassandra.components.CassandraModule;
import org.apache.james.lifecycle.api.Startable;
Expand All @@ -28,7 +26,6 @@
import org.apache.james.user.cassandra.CassandraUsersRepository;
import org.apache.james.utils.ConfigurationPerformer;

import com.google.common.collect.ImmutableList;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Scopes;
Expand Down Expand Up @@ -68,8 +65,8 @@ public void initModule() {
}

@Override
public List<Class<? extends Startable>> forClasses() {
return ImmutableList.of(CassandraUsersRepository.class);
public Class<? extends Startable> forClass() {
return CassandraUsersRepository.class;
}
}

Expand Down
Expand Up @@ -23,7 +23,6 @@

import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.List;

import javax.inject.Inject;
import javax.inject.Named;
Expand Down Expand Up @@ -53,7 +52,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.collect.ImmutableList;
import com.google.inject.AbstractModule;
import com.google.inject.Provides;
import com.google.inject.Scopes;
Expand Down Expand Up @@ -86,7 +84,6 @@ void createIndex() throws IOException {
}

static class ElasticSearchMailboxIndexCreationPerformer implements ConfigurationPerformer {

private final MailboxIndexCreator mailboxIndexCreator;

@Inject
Expand All @@ -104,8 +101,8 @@ public void initModule() {
}

@Override
public List<Class<? extends Startable>> forClasses() {
return ImmutableList.of(MailboxIndexCreator.class);
public Class<? extends Startable> forClass() {
return MailboxIndexCreator.class;
}
}

Expand Down
Expand Up @@ -23,7 +23,6 @@

import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.List;

import javax.inject.Inject;

Expand All @@ -45,7 +44,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.collect.ImmutableList;
import com.google.inject.AbstractModule;
import com.google.inject.Provides;
import com.google.inject.Singleton;
Expand Down Expand Up @@ -95,8 +93,8 @@ public void initModule() {
}

@Override
public List<Class<? extends Startable>> forClasses() {
return ImmutableList.of(ElasticSearchQuotaIndexCreator.class);
public Class<? extends Startable> forClass() {
return ElasticSearchQuotaIndexCreator.class;
}
}

Expand Down
Expand Up @@ -19,14 +19,11 @@

package org.apache.james.modules.metrics;

import java.util.List;

import org.apache.james.lifecycle.api.Startable;
import org.apache.james.utils.ConfigurationPerformer;

import com.codahale.metrics.MetricRegistry;
import com.datastax.driver.core.Session;
import com.google.common.collect.ImmutableList;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Scopes;
Expand All @@ -44,7 +41,7 @@ protected void configure() {
.to(CassandraMetricsInjector.class);
}

public static class CassandraMetricsInjector implements ConfigurationPerformer {
public static class CassandraMetricsInjector implements ConfigurationPerformer, Startable {

private final MetricRegistry metricRegistry;
private final Session session;
Expand All @@ -64,8 +61,8 @@ public void initModule() {
}

@Override
public List<Class<? extends Startable>> forClasses() {
return ImmutableList.of();
public Class<? extends Startable> forClass() {
return CassandraMetricsInjector.class;
}
}
}
Expand Up @@ -22,8 +22,6 @@
import static org.apache.james.CassandraJamesServerMain.ALL_BUT_JMX_CASSANDRA_MODULE;
import static org.assertj.core.api.Assertions.assertThatCode;

import java.util.List;

import javax.inject.Inject;

import org.apache.james.lifecycle.api.Startable;
Expand All @@ -35,7 +33,6 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import com.google.common.collect.ImmutableList;
import com.google.inject.multibindings.Multibinder;

class CassandraMessageIdManagerInjectionTest {
Expand All @@ -60,7 +57,7 @@ void messageIdManagerShouldBeInjected(GuiceJamesServer server) {
assertThatCode(server::start).doesNotThrowAnyException();
}

public static class CallMe implements ConfigurationPerformer {
public static class CallMe implements ConfigurationPerformer, Startable {
@Inject
public CallMe(MessageIdManager messageIdManager) {
}
Expand All @@ -71,8 +68,8 @@ public void initModule() {
}

@Override
public List<Class<? extends Startable>> forClasses() {
return ImmutableList.of();
public Class<? extends Startable> forClass() {
return CallMe.class;
}
}
}
Expand Up @@ -18,8 +18,6 @@
****************************************************************/
package org.apache.james.data;

import java.util.List;

import org.apache.commons.configuration2.ex.ConfigurationException;
import org.apache.james.lifecycle.api.Startable;
import org.apache.james.server.core.configuration.ConfigurationProvider;
Expand All @@ -28,7 +26,6 @@
import org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository;
import org.apache.james.utils.ConfigurationPerformer;

import com.google.common.collect.ImmutableList;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Provides;
Expand Down Expand Up @@ -76,8 +73,8 @@ public void initModule() {
}

@Override
public List<Class<? extends Startable>> forClasses() {
return ImmutableList.of(ReadOnlyUsersLDAPRepository.class);
public Class<? extends Startable> forClass() {
return ReadOnlyUsersLDAPRepository.class;
}
}

Expand Down
Expand Up @@ -19,8 +19,6 @@

package org.apache.james.modules.event;

import java.util.List;

import org.apache.james.event.json.EventSerializer;
import org.apache.james.lifecycle.api.Startable;
import org.apache.james.mailbox.events.EventBus;
Expand All @@ -29,7 +27,6 @@
import org.apache.james.mailbox.events.RegistrationKey;
import org.apache.james.mailbox.events.RetryBackoffConfiguration;
import org.apache.james.utils.ConfigurationPerformer;
import org.parboiled.common.ImmutableList;

import com.google.inject.AbstractModule;
import com.google.inject.Inject;
Expand Down Expand Up @@ -68,8 +65,8 @@ public void initModule() {
}

@Override
public List<Class<? extends Startable>> forClasses() {
return ImmutableList.of(RabbitMQEventBus.class);
public Class<? extends Startable> forClass() {
return RabbitMQEventBus.class;
}
}
}
Expand Up @@ -19,8 +19,6 @@

package org.apache.james.utils;

import java.util.List;

import org.apache.james.lifecycle.api.Startable;

public interface ConfigurationPerformer {
Expand All @@ -30,11 +28,9 @@ public interface ConfigurationPerformer {
/**
* In order to initialize components in the right order, every
* {@link ConfigurationPerformer} is supposed to declare which
* classes it will initialize.
* class it will initialize.
*
* @return the list of Classes that this object will initialize.
* The list should never be empty.
* @return the Class that this object will initialize.
*/
List<Class<? extends Startable>> forClasses();

Class<? extends Startable> forClass();
}
Expand Up @@ -20,7 +20,6 @@
package org.apache.james.modules.server;

import java.io.FileNotFoundException;
import java.util.List;

import org.apache.commons.configuration2.Configuration;
import org.apache.commons.configuration2.ex.ConfigurationException;
Expand All @@ -32,7 +31,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.collect.ImmutableList;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Provides;
Expand Down Expand Up @@ -87,7 +85,6 @@ private boolean isMetricEnable(Configuration propertiesReader) {

@Singleton
public static class ESMetricReporterStarter implements ConfigurationPerformer {

private final ESMetricReporter esMetricReporter;

@Inject
Expand All @@ -101,8 +98,8 @@ public void initModule() {
}

@Override
public List<Class<? extends Startable>> forClasses() {
return ImmutableList.of(ESMetricReporter.class);
public Class<? extends Startable> forClass() {
return ESMetricReporter.class;
}
}

Expand Down
Expand Up @@ -18,15 +18,12 @@
****************************************************************/
package org.apache.james.modules.server;

import java.util.List;

import org.apache.james.dnsservice.api.DNSService;
import org.apache.james.dnsservice.dnsjava.DNSJavaService;
import org.apache.james.lifecycle.api.Startable;
import org.apache.james.server.core.configuration.ConfigurationProvider;
import org.apache.james.utils.ConfigurationPerformer;

import com.google.common.collect.ImmutableList;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Scopes;
Expand Down Expand Up @@ -66,8 +63,8 @@ public void initModule() {
}

@Override
public List<Class<? extends Startable>> forClasses() {
return ImmutableList.of(DNSJavaService.class);
public Class<? extends Startable> forClass() {
return DNSJavaService.class;
}
}
}

0 comments on commit 7faac28

Please sign in to comment.