Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CHE-3064: Add log errors on pre-destroy phase #3990

Merged
merged 3 commits into from
Feb 11, 2017
Merged

CHE-3064: Add log errors on pre-destroy phase #3990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1ac58a0
CHE-3064: Add log errors on pre-destroy phase
Feb 1, 2017
05cf521
Merge branch 'master' of github.com:eclipse/che into CHE-3064
Feb 6, 2017
33a8015
fixup! CHE-3064: Add log errors on pre-destroy phase
Feb 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions core/commons/che-core-commons-inject/src/main/java/org/eclipse/che/inject/CheBootstrap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.google.inject.util.Modules;
import com.google.inject.util.Providers;

import org.eclipse.che.inject.lifecycle.DestroyErrorHandler;
import org.eclipse.che.inject.lifecycle.DestroyModule;
import org.eclipse.che.inject.lifecycle.Destroyer;
import org.eclipse.che.inject.lifecycle.InitModule;
Expand Down Expand Up @@ -49,9 +48,10 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static java.lang.String.format;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toMap;
import static java.util.stream.Collectors.toSet;
import static org.eclipse.che.inject.lifecycle.DestroyErrorHandler.LOG_HANDLER;

/**
* CheBootstrap is entry point of Che application implemented as ServletContextListener.
Expand Down Expand Up @@ -126,7 +126,7 @@ public void contextDestroyed(ServletContextEvent sce) {
protected List<Module> getModules() {
// based on logic that getServletModule() is called BEFORE getModules() in the EverrestGuiceContextListener
modules.add(new InitModule(PostConstruct.class));
modules.add(new DestroyModule(PreDestroy.class, DestroyErrorHandler.DUMMY));
modules.add(new DestroyModule(PreDestroy.class, LOG_HANDLER));
modules.add(new URIConverter());
modules.add(new URLConverter());
modules.add(new FileConverter());
Expand All @@ -137,8 +137,10 @@ protected List<Module> getModules() {
modules.addAll(ModuleScanner.findModules());
Map<String, Set<String>> aliases = readConfigurationAliases();
Module firstConfigurationPermutation = Modules.override(new WebInfConfiguration(aliases)).with(new ExtConfiguration(aliases));
Module secondConfigurationPermutation = Modules.override(firstConfigurationPermutation).with(new CheSystemPropertiesConfigurationModule(aliases));
Module lastConfigurationPermutation = Modules.override(secondConfigurationPermutation).with(new CheEnvironmentVariablesConfigurationModule(aliases));
Module secondConfigurationPermutation = Modules.override(firstConfigurationPermutation)
.with(new CheSystemPropertiesConfigurationModule(aliases));
Module lastConfigurationPermutation = Modules.override(secondConfigurationPermutation)
.with(new CheEnvironmentVariablesConfigurationModule(aliases));
modules.add(lastConfigurationPermutation);
return modules;
}
Expand All @@ -152,7 +154,7 @@ private Map<String, Set<String>> readConfigurationAliases() {
try (Reader reader = Files.newReader(aliasesFile, Charset.forName("UTF-8"))) {
properties.load(reader);
} catch (IOException e) {
throw new IllegalStateException(String.format("Unable to read configuration aliases from file %s", aliasesFile), e);
throw new IllegalStateException(format("Unable to read configuration aliases from file %s", aliasesFile), e);
}
for (Map.Entry<Object, Object> entry : properties.entrySet()) {
String value = (String)entry.getValue();
Expand Down Expand Up @@ -268,7 +270,8 @@ public Map.Entry<String, V> apply(Map.Entry<K, V> entry) {
}
}

static class EnvironmentVariableToSystemPropertyFormatNameConverter implements Function<Map.Entry<String, String>, Map.Entry<String, String>> {
static class EnvironmentVariableToSystemPropertyFormatNameConverter
implements Function<Map.Entry<String, String>, Map.Entry<String, String>> {
@Override
public Map.Entry<String, String> apply(Map.Entry<String, String> entry) {
String name = entry.getKey();
Expand Down Expand Up @@ -303,7 +306,7 @@ protected void bindConf(File confDir) {
try (Reader reader = Files.newReader(file, Charset.forName("UTF-8"))) {
properties.load(reader);
} catch (IOException e) {
throw new IllegalStateException(String.format("Unable to read configuration file %s", file), e);
throw new IllegalStateException(format("Unable to read configuration file %s", file), e);
}
bindProperties(properties);
}
Expand Down Expand Up @@ -353,10 +356,11 @@ protected <K, V> void bindProperties(String prefix, Iterable<Map.Entry<K, V>> pr
buf.append(resolvedPlaceholder);
} else if (skipUnresolved) {
buf.append(placeholder);
LOG.warn("Placeholder {} cannot be resolved neither from environment variable nor from system property, leaving as is.", placeholderName);
LOG.warn("Placeholder {} cannot be resolved neither from environment variable nor from system property," +
"leaving as is.", placeholderName);
} else {
throw new ConfigurationException(
String.format("Property %s is not found as system property or environment variable.", placeholderName));
throw new ConfigurationException(format("Property %s is not found as system property or " +
"environment variable.", placeholderName));
}

start = matcher.end();
Expand All @@ -376,7 +380,8 @@ private void bindProperty(String prefix, String name, String value) {
bind(String.class).annotatedWith(Names.named(key)).toProvider(Providers.<String>of(null));
if (aliasesForName != null) {
for (String alias : aliasesForName) {
bind(String.class).annotatedWith(Names.named(prefix == null ? alias : prefix + alias)).toProvider(Providers.<String>of(null));
bind(String.class).annotatedWith(Names.named(prefix == null ? alias : prefix + alias))
.toProvider(Providers.<String>of(null));
}
}
} else {
Expand Down
11 changes: 4 additions & 7 deletions ...re-commons-inject/src/main/java/org/eclipse/che/inject/lifecycle/DestroyErrorHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

import java.lang.reflect.Method;

import static org.slf4j.LoggerFactory.getLogger;

/**
* Helps to be more flexible when need handle errors of invocation destroy-methods.
*
Expand All @@ -21,12 +23,7 @@ public interface DestroyErrorHandler {
void onError(Object instance, Method method, Throwable error);

/**
* Implementation of DestroyErrorHandler that ignore errors, e.g. such behaviour is required for annotation {@link
* javax.annotation.PreDestroy}.
* Implementation of DestroyErrorHandler that log errors.
*/
DestroyErrorHandler DUMMY = new DestroyErrorHandler() {
@Override
public void onError(Object instance, Method method, Throwable error) {
}
};
DestroyErrorHandler LOG_HANDLER = (instance, method, error) -> getLogger(instance.getClass()).error(error.getMessage(), error);
}
5 changes: 3 additions & 2 deletions core/commons/che-core-commons-inject/src/test/java/org/eclipse/che/inject/LifecycleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
*******************************************************************************/
package org.eclipse.che.inject;

import org.eclipse.che.inject.lifecycle.DestroyErrorHandler;
import org.eclipse.che.inject.lifecycle.DestroyModule;
import org.eclipse.che.inject.lifecycle.Destroyer;
import org.eclipse.che.inject.lifecycle.InitModule;
Expand All @@ -28,14 +27,16 @@
import javax.inject.Inject;
import javax.inject.Singleton;

import static org.eclipse.che.inject.lifecycle.DestroyErrorHandler.LOG_HANDLER;

/** @author andrew00x */
public class LifecycleTest {
Injector injector;

@BeforeTest
public void init() {
injector = Guice.createInjector(new InitModule(PostConstruct.class),
new DestroyModule(PreDestroy.class, DestroyErrorHandler.DUMMY),
new DestroyModule(PreDestroy.class, LOG_HANDLER),
new MyModule());
}

Expand Down