GEODE-2601: Made banner and configs log only once and added a test to verify changes
Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/c5520f95 Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/c5520f95 Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/c5520f95 Branch: refs/heads/feature/GEM-1483 Commit: c5520f95635ebc3493aa4246e975f1f7e4aac828 Parents: 0bce1ea Author: YehEmily <emilyyeh1...@gmail.com> Authored: Tue Jun 27 12:56:40 2017 -0700 Committer: YehEmily <emilyyeh1...@gmail.com> Committed: Mon Jul 17 11:31:17 2017 -0700 ---------------------------------------------------------------------- .../cli/commands/GfshStartLocatorLogTest.java | 61 ++++++++++++++++++++ .../distributed/internal/InternalLocator.java | 6 -- .../apache/geode/internal/AbstractConfig.java | 18 +++--- .../java/org/apache/geode/internal/Banner.java | 8 +-- .../geode/internal/i18n/LocalizedStrings.java | 2 +- .../internal/logging/LogWriterFactory.java | 2 +- .../logging/log4j/LogWriterAppenders.java | 8 +-- .../internal/logging/TestLogWriterFactory.java | 4 +- .../geode/test/dunit/SerializableCallable.java | 2 +- 9 files changed, 85 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/c5520f95/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/GfshStartLocatorLogTest.java ---------------------------------------------------------------------- diff --git a/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/GfshStartLocatorLogTest.java b/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/GfshStartLocatorLogTest.java new file mode 100644 index 0000000..eadbea8 --- /dev/null +++ b/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/GfshStartLocatorLogTest.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.geode.management.internal.cli.commands; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.File; +import java.nio.charset.StandardCharsets; + +import com.google.common.io.Files; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import org.apache.geode.internal.AbstractConfig; +import org.apache.geode.internal.Banner; +import org.apache.geode.internal.i18n.LocalizedStrings; +import org.apache.geode.internal.logging.log4j.LocalizedMessage; +import org.apache.geode.test.dunit.rules.gfsh.GfshExecution; +import org.apache.geode.test.dunit.rules.gfsh.GfshRule; +import org.apache.geode.test.dunit.rules.gfsh.GfshScript; +import org.apache.geode.test.junit.categories.DistributedTest; + +@Category(DistributedTest.class) +public class GfshStartLocatorLogTest { + @Rule + public GfshRule gfshRule = new GfshRule(); + + @Test + public void bannerOnlyLogsOnce() throws Exception { + final String banner = "Licensed to the Apache Software Foundation (ASF)"; + String lines = getExecutionLogs(); + assertThat(lines.indexOf(banner)).isEqualTo(lines.lastIndexOf(banner)); + } + + @Test + public void startupConfigsOnlyLogsOnce() throws Exception { + final String startupConfigs = AbstractConfig.GEM_FIRE_PROPERTIES_USING_DEFAULT_VALUES; + String lines = getExecutionLogs(); + assertThat(lines.indexOf(startupConfigs)).isEqualTo(lines.lastIndexOf(startupConfigs)); + } + + private String getExecutionLogs() throws Exception { + GfshExecution gfshExecution = GfshScript.of("start locator").execute(gfshRule); + File[] files = gfshExecution.getWorkingDir().listFiles(); + String logName = files[0].getAbsolutePath() + "/" + files[0].getName() + ".log"; + return Files.readLines(new File(logName), StandardCharsets.UTF_8).toString(); + } +} http://git-wip-us.apache.org/repos/asf/geode/blob/c5520f95/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java index 6eaaec2..ffe3be4 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java @@ -675,7 +675,6 @@ public class InternalLocator extends Locator implements ConnectListener { System.setProperty(propName, locatorsProp); } } - // No longer default mcast-port to zero. See 46277. } @@ -686,11 +685,6 @@ public class InternalLocator extends Locator implements ConnectListener { logger.info( LocalizedMessage.create(LocalizedStrings.InternalLocator_STARTING_DISTRIBUTED_SYSTEM)); - // LOG:CONFIG: changed from config to info - logger.info(LogMarker.CONFIG, - LocalizedMessage.create( - LocalizedStrings.InternalDistributedSystem_STARTUP_CONFIGURATIONN_0, - this.config.toLoggerString())); this.myDs = (InternalDistributedSystem) DistributedSystem.connect(connectEnv); http://git-wip-us.apache.org/repos/asf/geode/blob/c5520f95/geode-core/src/main/java/org/apache/geode/internal/AbstractConfig.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/AbstractConfig.java b/geode-core/src/main/java/org/apache/geode/internal/AbstractConfig.java index 7bb2de9..9b9c0da 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/AbstractConfig.java +++ b/geode-core/src/main/java/org/apache/geode/internal/AbstractConfig.java @@ -49,6 +49,12 @@ import org.apache.geode.internal.security.SecurableCommunicationChannel; */ public abstract class AbstractConfig implements Config { + public static final String GEM_FIRE_PROPERTIES_USING_DEFAULT_VALUES = + "### GemFire Properties using default values ###"; + public static final String GEM_FIRE_PROPERTIES_DEFINED_WITH_PREFIX = + "### GemFire Properties defined with "; + public static final String GEM_FIRE_PROPERTIES_DEFINED_WITH_SUFFIX = " ###"; + /** * Returns the string to use as the exception message when an attempt is made to set an * unmodifiable attribute. @@ -173,9 +179,10 @@ public abstract class AbstractConfig implements Config { if (!sourceFound) { sourceFound = true; if (source == null) { - pw.println("### GemFire Properties using default values ###"); + pw.println(GEM_FIRE_PROPERTIES_USING_DEFAULT_VALUES); } else { - pw.println("### GemFire Properties defined with " + source.getDescription() + " ###"); + pw.println(GEM_FIRE_PROPERTIES_DEFINED_WITH_PREFIX + source.getDescription() + + GEM_FIRE_PROPERTIES_DEFINED_WITH_SUFFIX); } } // hide the shiro-init configuration for now. Remove after we can allow customer to specify @@ -205,10 +212,7 @@ public abstract class AbstractConfig implements Config { if (attName.startsWith(DistributionConfig.SYS_PROP_NAME)) { return false; } - if (attName.toLowerCase().contains("password") /* bug 45381 */) { - return false; - } - return true; + return !attName.toLowerCase().contains("password"); } /** @@ -388,7 +392,7 @@ public abstract class AbstractConfig implements Config { String trimAttName = trimAttributeName(attName); throw new UnmodifiableException( LocalizedStrings.AbstractConfig_THE_0_CONFIGURATION_ATTRIBUTE_CAN_NOT_BE_SET_FROM_THE_COMMAND_LINE_SET_1_FOR_EACH_INDIVIDUAL_PARAMETER_INSTEAD - .toLocalizedString(new Object[] {attName, trimAttName})); + .toLocalizedString(attName, trimAttName)); } } else if (valueType.equals(FlowControlParams.class)) { String values[] = attValue.split(","); http://git-wip-us.apache.org/repos/asf/geode/blob/c5520f95/geode-core/src/main/java/org/apache/geode/internal/Banner.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/Banner.java b/geode-core/src/main/java/org/apache/geode/internal/Banner.java index a218a5b..38b3a31 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/Banner.java +++ b/geode-core/src/main/java/org/apache/geode/internal/Banner.java @@ -59,7 +59,7 @@ public class Banner { static void print(PrintWriter out, String args[]) { Map sp = new TreeMap((Properties) System.getProperties().clone()); // fix for 46822 int processId = -1; - final String SEPERATOR = + final String SEPARATOR = "---------------------------------------------------------------------------"; try { processId = OSProcess.getId(); @@ -80,7 +80,7 @@ public class Banner { final String productName = GemFireVersion.getProductName(); - out.println(SEPERATOR); + out.println(SEPARATOR); out.println(" "); out.println(" Licensed to the Apache Software Foundation (ASF) under one or more"); @@ -100,7 +100,7 @@ public class Banner { out.println(" under the License."); out.println(" "); - out.println(SEPERATOR); + out.println(SEPARATOR); GemFireVersion.print(out); @@ -153,7 +153,7 @@ public class Banner { out.println("Log4J 2 Configuration:"); out.println(" " + LogService.getConfigInformation()); } - out.println(SEPERATOR); + out.println(SEPARATOR); } /** http://git-wip-us.apache.org/repos/asf/geode/blob/c5520f95/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java b/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java index baad039..3a32db8 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java +++ b/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java @@ -1112,7 +1112,7 @@ public class LocalizedStrings { new StringId(1690, "Running in local mode since no locators were specified."); public static final StringId InternalDistributedSystem_SHUTDOWNLISTENER__0__THREW = new StringId(1691, "ShutdownListener < {0} > threw..."); - public static final StringId InternalDistributedSystem_STARTUP_CONFIGURATIONN_0 = + public static final StringId InternalDistributedSystem_STARTUP_CONFIGURATION_0 = new StringId(1692, "Startup Configuration:\n {0}"); public static final StringId InternalInstantiator_CLASS_0_DOES_NOT_HAVE_A_TWOARGUMENT_CLASS_INT_CONSTRUCTOR = http://git-wip-us.apache.org/repos/asf/geode/blob/c5520f95/geode-core/src/main/java/org/apache/geode/internal/logging/LogWriterFactory.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/logging/LogWriterFactory.java b/geode-core/src/main/java/org/apache/geode/internal/logging/LogWriterFactory.java index 4202afd..e8c283c 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/logging/LogWriterFactory.java +++ b/geode-core/src/main/java/org/apache/geode/internal/logging/LogWriterFactory.java @@ -95,7 +95,7 @@ public class LogWriterFactory { // LOG:CONFIG: changed from config to info logger.info(LogMarker.CONFIG, LocalizedMessage.create( - LocalizedStrings.InternalDistributedSystem_STARTUP_CONFIGURATIONN_0, + LocalizedStrings.InternalDistributedSystem_STARTUP_CONFIGURATION_0, config.toLoggerString())); } return logger; http://git-wip-us.apache.org/repos/asf/geode/blob/c5520f95/geode-core/src/main/java/org/apache/geode/internal/logging/log4j/LogWriterAppenders.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/logging/log4j/LogWriterAppenders.java b/geode-core/src/main/java/org/apache/geode/internal/logging/log4j/LogWriterAppenders.java index 310deb2..a500654 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/logging/log4j/LogWriterAppenders.java +++ b/geode-core/src/main/java/org/apache/geode/internal/logging/log4j/LogWriterAppenders.java @@ -40,14 +40,14 @@ public class LogWriterAppenders { MAIN(false), SECURITY(true); private final boolean isSecure; - private Identifier(final boolean isSecure) { + Identifier(final boolean isSecure) { this.isSecure = isSecure; } public boolean isSecure() { return this.isSecure; } - }; + } private static Map<Identifier, LogWriterAppender> appenders = new HashMap<Identifier, LogWriterAppender>(); @@ -151,7 +151,7 @@ public class LogWriterAppenders { } else { firstMsgWarning = true; firstMsg = LocalizedStrings.InternalDistributedSystem_COULD_NOT_RENAME_0_TO_1 - .toLocalizedString(new Object[] {logFile, oldMain}); + .toLocalizedString(logFile, oldMain); } } } @@ -220,7 +220,7 @@ public class LogWriterAppenders { if (logConfig) { if (!isLoner) { // LOG:CONFIG: changed from config to info - logWriter.info(LocalizedStrings.InternalDistributedSystem_STARTUP_CONFIGURATIONN_0, + logWriter.info(LocalizedStrings.InternalDistributedSystem_STARTUP_CONFIGURATION_0, config.toLoggerString()); } } http://git-wip-us.apache.org/repos/asf/geode/blob/c5520f95/geode-core/src/test/java/org/apache/geode/internal/logging/TestLogWriterFactory.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/internal/logging/TestLogWriterFactory.java b/geode-core/src/test/java/org/apache/geode/internal/logging/TestLogWriterFactory.java index 87f370c..17fdd05 100644 --- a/geode-core/src/test/java/org/apache/geode/internal/logging/TestLogWriterFactory.java +++ b/geode-core/src/test/java/org/apache/geode/internal/logging/TestLogWriterFactory.java @@ -71,7 +71,7 @@ public class TestLogWriterFactory extends Assert { } else { firstMsgWarning = true; firstMsg = LocalizedStrings.InternalDistributedSystem_COULD_NOT_RENAME_0_TO_1 - .toLocalizedString(new Object[] {logFile, oldMain}); + .toLocalizedString(logFile, oldMain); } } } @@ -114,7 +114,7 @@ public class TestLogWriterFactory extends Assert { } if (logConfig && logger.configEnabled()) { logger.convertToLogWriterI18n().config( - LocalizedStrings.InternalDistributedSystem_STARTUP_CONFIGURATIONN_0, + LocalizedStrings.InternalDistributedSystem_STARTUP_CONFIGURATION_0, config.toLoggerString()); } http://git-wip-us.apache.org/repos/asf/geode/blob/c5520f95/geode-core/src/test/java/org/apache/geode/test/dunit/SerializableCallable.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/test/dunit/SerializableCallable.java b/geode-core/src/test/java/org/apache/geode/test/dunit/SerializableCallable.java index 68d5229..29b083a 100644 --- a/geode-core/src/test/java/org/apache/geode/test/dunit/SerializableCallable.java +++ b/geode-core/src/test/java/org/apache/geode/test/dunit/SerializableCallable.java @@ -22,7 +22,7 @@ import java.util.concurrent.Callable; * conjunction with {@link VM#invoke(SerializableCallableIF)}. * * <PRE> - * public void testRepilcatedRegionPut() { + * public void testReplicatedRegionPut() { * final Host host = Host.getHost(0); * VM vm0 = host.getVM(0); * VM vm1 = host.getVM(1);
