Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 0AF75200D0E for ; Tue, 12 Sep 2017 04:24:51 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 096AE1609C4; Tue, 12 Sep 2017 02:24:51 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 562341609C6 for ; Tue, 12 Sep 2017 04:24:49 +0200 (CEST) Received: (qmail 27529 invoked by uid 500); 12 Sep 2017 02:24:48 -0000 Mailing-List: contact commits-help@accumulo.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@accumulo.apache.org Delivered-To: mailing list commits@accumulo.apache.org Received: (qmail 27519 invoked by uid 99); 12 Sep 2017 02:24:48 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 12 Sep 2017 02:24:48 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id E40B081752; Tue, 12 Sep 2017 02:24:46 +0000 (UTC) Date: Tue, 12 Sep 2017 02:24:49 +0000 To: "commits@accumulo.apache.org" Subject: [accumulo] 03/03: ACCUMULO-4086 Make chooser classes more testable MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit From: ctubbsii@apache.org Reply-To: "commits@accumulo.apache.org" In-Reply-To: <150518308663.4557.16634047853334636858@gitbox.apache.org> References: <150518308663.4557.16634047853334636858@gitbox.apache.org> X-Git-Host: gitbox.apache.org X-Git-Repo: accumulo X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Rev: 21fb47733416ec8690e364420c41d2e7b731cda7 X-Git-NotificationType: diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated Message-Id: <20170912022446.E40B081752@gitbox.apache.org> archived-at: Tue, 12 Sep 2017 02:24:51 -0000 This is an automated email from the ASF dual-hosted git repository. ctubbsii pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/accumulo.git commit 21fb47733416ec8690e364420c41d2e7b731cda7 Author: Christopher Tubbs AuthorDate: Mon Sep 11 21:48:41 2017 -0400 ACCUMULO-4086 Make chooser classes more testable Improve the testability of PerTableVolumeChooser and PreferredVolumeChooser, and dramatically simplify their unit tests. PerTableVolumeChooser.choose becomes: getDelegate(env).choose(env, options); And PerTableVolumeChooserTest tests getDelegate(env) PreferredVolumeChooser.choose becomes: super.choose(env, getPreferredVolumes(env, options)); And PreferredVolumesChooserTest tests getPreferredVolumes(env, options) --- .../accumulo/server/fs/PerTableVolumeChooser.java | 50 ++-- .../accumulo/server/fs/PreferredVolumeChooser.java | 33 ++- .../server/fs/PerTableVolumeChooserTest.java | 281 +++++++-------------- .../server/fs/PreferredVolumeChooserTest.java | 251 ++++++++---------- 4 files changed, 233 insertions(+), 382 deletions(-) diff --git a/server/base/src/main/java/org/apache/accumulo/server/fs/PerTableVolumeChooser.java b/server/base/src/main/java/org/apache/accumulo/server/fs/PerTableVolumeChooser.java index 6eba22c..ab642d6 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/fs/PerTableVolumeChooser.java +++ b/server/base/src/main/java/org/apache/accumulo/server/fs/PerTableVolumeChooser.java @@ -58,22 +58,21 @@ public class PerTableVolumeChooser implements VolumeChooser { @Override public String choose(VolumeChooserEnvironment env, String[] options) throws VolumeChooserException { log.trace("{}.choose", getClass().getSimpleName()); + return getDelegateChooser(env).choose(env, options); + } - VolumeChooser delegateChooser; + // visible (not private) for testing + VolumeChooser getDelegateChooser(VolumeChooserEnvironment env) { switch (env.getScope()) { case INIT: // TODO should be possible to read from SiteConfiguration during init - log.warn("Not possible to determine delegate chooser at '{}' scope. Using all volumes.", ChooserScope.INIT); - delegateChooser = randomChooser; - break; + log.warn("Not possible to determine delegate chooser at '{}' scope. Using {}.", ChooserScope.INIT, RandomVolumeChooser.class.getName()); + return randomChooser; case TABLE: - delegateChooser = getVolumeChooserForTable(env, loadConfFactory()); - break; + return getVolumeChooserForTable(env, loadConfFactory()); default: - delegateChooser = getVolumeChooserForScope(env, loadConfFactory()); - break; + return getVolumeChooserForScope(env, loadConfFactory()); } - return delegateChooser.choose(env, options); } private VolumeChooser getVolumeChooserForTable(VolumeChooserEnvironment env, ServerConfigurationFactory confFactory) { @@ -92,10 +91,15 @@ public class PerTableVolumeChooser implements VolumeChooser { throw new VolumeChooserException(msg); } - String context = tableConf.get(Property.TABLE_CLASSPATH); // can be null + String context = getTableContext(tableConf); // can be null return createVolumeChooser(context, clazz, TABLE_VOLUME_CHOOSER, env.getTableId(), tableSpecificChooserCache); } + // visible (not private) for testing + String getTableContext(TableConfiguration tableConf) { + return tableConf.get(Property.TABLE_CLASSPATH); + } + private VolumeChooser getVolumeChooserForScope(VolumeChooserEnvironment env, ServerConfigurationFactory confFactory) { ChooserScope scope = env.getScope(); String property = getPropertyNameForScope(scope); @@ -142,22 +146,22 @@ public class PerTableVolumeChooser implements VolumeChooser { if (previousChooser != null && previousChooser.getClass().getName().equals(className)) { // no change; return the old one return previousChooser; + } else if (previousChooser == null) { + // TODO stricter definition of when the updated property is used, ref ACCUMULO-3412 + // don't log change if this is the first use + log.trace("Change detected for {} for {}", property, key); + } + try { + return ConfigurationTypeHelper.getClassInstance(context, className, VolumeChooser.class); + } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | IOException e) { + String msg = "Failed to create instance for " + key + " configured to use " + className + " via " + property; + throw new VolumeChooserException(msg, e); } - // TODO stricter definition of when the updated property is used, ref ACCUMULO-3412 - if (previousChooser == null) { - // don't log change if this is the first use - log.trace("Change detected for {} for {}", property, key); - } - try { - return ConfigurationTypeHelper.getClassInstance(context, className, VolumeChooser.class); - } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | IOException e) { - String msg = "Failed to create instance for " + key + " configured to use " + className + " via " + property; - throw new VolumeChooserException(msg, e); - } - }); + }); } - private ServerConfigurationFactory loadConfFactory() { + // visible (not private) for testing + ServerConfigurationFactory loadConfFactory() { // This local variable is an intentional component of the single-check idiom. ServerConfigurationFactory localConf = lazyConfFactory; if (localConf == null) { diff --git a/server/base/src/main/java/org/apache/accumulo/server/fs/PreferredVolumeChooser.java b/server/base/src/main/java/org/apache/accumulo/server/fs/PreferredVolumeChooser.java index 8bf5f2a..e1a9b12 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/fs/PreferredVolumeChooser.java +++ b/server/base/src/main/java/org/apache/accumulo/server/fs/PreferredVolumeChooser.java @@ -53,29 +53,27 @@ public class PreferredVolumeChooser extends RandomVolumeChooser { @Override public String choose(VolumeChooserEnvironment env, String[] options) throws VolumeChooserException { log.trace("{}.choose", getClass().getSimpleName()); + // Randomly choose the volume from the preferred volumes + String choice = super.choose(env, getPreferredVolumes(env, options)); + log.trace("Choice = {}", choice); + return choice; + } - Set preferredVolumes; + // visible (not private) for testing + String[] getPreferredVolumes(VolumeChooserEnvironment env, String[] options) { switch (env.getScope()) { case INIT: // TODO should be possible to read from SiteConfiguration during init log.warn("Not possible to determine preferred volumes at '{}' scope. Using all volumes.", ChooserScope.INIT); - return super.choose(env, options); + return options; case TABLE: - preferredVolumes = getPreferredVolumesForTable(env, loadConfFactory(), options); - break; + return getPreferredVolumesForTable(env, loadConfFactory(), options); default: - preferredVolumes = getPreferredVolumesForScope(env, loadConfFactory(), options); - break; + return getPreferredVolumesForScope(env, loadConfFactory(), options); } - - // Randomly choose the volume from the preferred volumes - String choice = super.choose(env, preferredVolumes.toArray(new String[preferredVolumes.size()])); - log.trace("Choice = {}", choice); - - return choice; } - private Set getPreferredVolumesForTable(VolumeChooserEnvironment env, ServerConfigurationFactory confFactory, String[] options) { + private String[] getPreferredVolumesForTable(VolumeChooserEnvironment env, ServerConfigurationFactory confFactory, String[] options) { log.trace("Looking up property {} + for Table id: {}", TABLE_PREFERRED_VOLUMES, env.getTableId()); final TableConfiguration tableConf = confFactory.getTableConfiguration(env.getTableId()); @@ -96,7 +94,7 @@ public class PreferredVolumeChooser extends RandomVolumeChooser { return parsePreferred(TABLE_PREFERRED_VOLUMES, preferredVolumes, options); } - private Set getPreferredVolumesForScope(VolumeChooserEnvironment env, ServerConfigurationFactory confFactory, String[] options) { + private String[] getPreferredVolumesForScope(VolumeChooserEnvironment env, ServerConfigurationFactory confFactory, String[] options) { ChooserScope scope = env.getScope(); String property = getPropertyNameForScope(scope); log.trace("Looking up property {} for scope: {}", property, scope); @@ -122,7 +120,7 @@ public class PreferredVolumeChooser extends RandomVolumeChooser { return parsePreferred(property, preferredVolumes, options); } - private Set parsePreferred(String property, String preferredVolumes, String[] options) { + private String[] parsePreferred(String property, String preferredVolumes, String[] options) { log.trace("Found {} = {}", property, preferredVolumes); Set preferred = Arrays.stream(StringUtils.split(preferredVolumes, ',')).map(String::trim).collect(Collectors.toSet()); @@ -137,10 +135,11 @@ public class PreferredVolumeChooser extends RandomVolumeChooser { throw new VolumeChooserException(msg); } - return preferred; + return preferred.toArray(new String[preferred.size()]); } - private ServerConfigurationFactory loadConfFactory() { + // visible (not private) for testing + ServerConfigurationFactory loadConfFactory() { // Get the current table's properties, and find the preferred volumes property // This local variable is an intentional component of the single-check idiom. ServerConfigurationFactory localConf = lazyConfFactory; diff --git a/server/base/src/test/java/org/apache/accumulo/server/fs/PerTableVolumeChooserTest.java b/server/base/src/test/java/org/apache/accumulo/server/fs/PerTableVolumeChooserTest.java index 3d1a3f1..6f6ba52 100644 --- a/server/base/src/test/java/org/apache/accumulo/server/fs/PerTableVolumeChooserTest.java +++ b/server/base/src/test/java/org/apache/accumulo/server/fs/PerTableVolumeChooserTest.java @@ -16,265 +16,164 @@ */ package org.apache.accumulo.server.fs; -import java.lang.reflect.Field; -import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.createStrictMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.fail; import org.apache.accumulo.core.client.impl.Table; import org.apache.accumulo.core.conf.AccumuloConfiguration; -import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.server.conf.ServerConfigurationFactory; import org.apache.accumulo.server.conf.TableConfiguration; import org.apache.accumulo.server.fs.VolumeChooser.VolumeChooserException; import org.apache.accumulo.server.fs.VolumeChooserEnvironment.ChooserScope; -import org.easymock.EasyMock; -import org.easymock.IExpectationSetters; -import org.junit.Assert; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import com.google.common.collect.Sets; - public class PerTableVolumeChooserTest { - private static final int REQUIRED_NUMBER_TRIES = 20; // times to call choose for likely exercising of each preferred volume - private static final String[] ALL_OPTIONS = new String[] {"1", "2", "3"}; - public static final String INVALID_CHOOSER_CLASSNAME = "MysteriousVolumeChooser"; - private ServerConfigurationFactory mockedServerConfigurationFactory; - private TableConfiguration mockedTableConfiguration; - private PerTableVolumeChooser perTableVolumeChooser; - private AccumuloConfiguration mockedAccumuloConfiguration; + + private ServerConfigurationFactory confFactory; + private TableConfiguration tableConf; + private PerTableVolumeChooser chooser; + private AccumuloConfiguration systemConf; + + public static class MockChooser1 extends RandomVolumeChooser {} + + public static class MockChooser2 extends RandomVolumeChooser {} @Rule public ExpectedException thrown = ExpectedException.none(); @Before public void before() throws Exception { - perTableVolumeChooser = new PerTableVolumeChooser(); - - mockedServerConfigurationFactory = EasyMock.createMock(ServerConfigurationFactory.class); - Field field = perTableVolumeChooser.getClass().getDeclaredField("lazyConfFactory"); - field.setAccessible(true); - field.set(perTableVolumeChooser, mockedServerConfigurationFactory); - - mockedTableConfiguration = EasyMock.createMock(TableConfiguration.class); - mockedAccumuloConfiguration = EasyMock.createMock(AccumuloConfiguration.class); - EasyMock.expect(mockedServerConfigurationFactory.getTableConfiguration(EasyMock. anyObject())).andReturn(mockedTableConfiguration).anyTimes(); - EasyMock.expect(mockedServerConfigurationFactory.getSystemConfiguration()).andReturn(mockedAccumuloConfiguration).anyTimes(); - EasyMock.expect(mockedTableConfiguration.get(Property.TABLE_CLASSPATH)).andReturn(null).anyTimes(); - } - - private IExpectationSetters expectDefaultScope(String className) { - return expectScope(ChooserScope.DEFAULT, className); - } + confFactory = createStrictMock(ServerConfigurationFactory.class); - private IExpectationSetters expectLoggerScope(String className) { - return expectScope(ChooserScope.LOGGER, className); - } - - private IExpectationSetters expectScope(ChooserScope scope, String className) { - return EasyMock.expect(mockedAccumuloConfiguration.get(PerTableVolumeChooser.getPropertyNameForScope(scope))).andReturn(className); - } - - private IExpectationSetters expectTableChooser(String className) { - return EasyMock.expect(mockedTableConfiguration.get(PerTableVolumeChooser.TABLE_VOLUME_CHOOSER)).andReturn(className); - } - - private Set chooseRepeatedlyForTable() { - VolumeChooserEnvironment volumeChooserEnvironment = new VolumeChooserEnvironment(Table.ID.of("h")); - Set results = new HashSet<>(); - for (int i = 0; i < REQUIRED_NUMBER_TRIES; i++) { - results.add(perTableVolumeChooser.choose(volumeChooserEnvironment, ALL_OPTIONS)); - } - return results; - } - - public static class VolumeChooserAlwaysOne extends VolumeChooserForFixedVolume { - public VolumeChooserAlwaysOne() { - super("1"); - } - } - - public static class VolumeChooserAlwaysTwo extends VolumeChooserForFixedVolume { - public VolumeChooserAlwaysTwo() { - super("2"); - } - } - - public static class VolumeChooserAlwaysThree extends VolumeChooserForFixedVolume { - public VolumeChooserAlwaysThree() { - super("3"); - } - } - - public static class VolumeChooserForFixedVolume implements VolumeChooser { - private final String onlyValidOption; - - public VolumeChooserForFixedVolume(String fixedVolume) { - onlyValidOption = fixedVolume; - } + chooser = new PerTableVolumeChooser() { + @Override + ServerConfigurationFactory loadConfFactory() { + return confFactory; + } - @Override - public String choose(VolumeChooserEnvironment env, String[] options) { - for (String option : options) { - if (onlyValidOption.equals(option)) { - return onlyValidOption; - } + @Override + String getTableContext(TableConfiguration tableConf) { + return null; } - return null; - } - } + }; - private Set chooseRepeatedlyForLogger() { - return chooseRepeatedlyForScope(ChooserScope.LOGGER); + tableConf = createStrictMock(TableConfiguration.class); + systemConf = createStrictMock(AccumuloConfiguration.class); + expect(confFactory.getTableConfiguration(anyObject())).andReturn(tableConf).anyTimes(); + expect(confFactory.getSystemConfiguration()).andReturn(systemConf).anyTimes(); } - private Set chooseRepeatedlyForScope(ChooserScope scope) { - VolumeChooserEnvironment volumeChooserEnvironment = new VolumeChooserEnvironment(scope); - Set results = new HashSet<>(); - - for (int i = 0; i < REQUIRED_NUMBER_TRIES; i++) { - results.add(perTableVolumeChooser.choose(volumeChooserEnvironment, ALL_OPTIONS)); - } - return results; + @After + public void after() throws Exception { + verify(confFactory, tableConf, systemConf); } - @Test - public void testInitScope() throws Exception { - VolumeChooserEnvironment volumeChooserEnvironment = new VolumeChooserEnvironment(ChooserScope.INIT); - - Set results = new HashSet<>(); - for (int i = 0; i < REQUIRED_NUMBER_TRIES; i++) { - results.add(perTableVolumeChooser.choose(volumeChooserEnvironment, ALL_OPTIONS)); - } - - Assert.assertEquals(Sets.newHashSet(Arrays.asList(ALL_OPTIONS)), results); + private VolumeChooser getTableDelegate() { + VolumeChooserEnvironment env = new VolumeChooserEnvironment(Table.ID.of("testTable")); + return chooser.getDelegateChooser(env); } - @Test - public void testTableConfig() throws Exception { - expectTableChooser(VolumeChooserAlwaysTwo.class.getName()).atLeastOnce(); - - EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration); - - Set results = chooseRepeatedlyForTable(); - - EasyMock.verify(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration); - Assert.assertEquals(Sets.newHashSet(Arrays.asList("2")), results); + private VolumeChooser getDelegate(ChooserScope scope) { + VolumeChooserEnvironment env = new VolumeChooserEnvironment(scope); + return chooser.getDelegateChooser(env); } @Test - public void testTableMisconfigured() throws Exception { - expectDefaultScope(VolumeChooserAlwaysOne.class.getName()); - expectTableChooser(INVALID_CHOOSER_CLASSNAME); - - EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration); - - thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForTable(); + public void testInitScopeSelectsRandomChooser() throws Exception { + replay(confFactory, tableConf, systemConf); + VolumeChooser delegate = getDelegate(ChooserScope.INIT); + assertSame(RandomVolumeChooser.class, delegate.getClass()); } @Test - public void testTableMissing() throws Exception { - expectDefaultScope(null); - expectTableChooser(null); - - EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration); + public void testTableScopeUsingTableProperty() throws Exception { + expect(tableConf.get(PerTableVolumeChooser.TABLE_VOLUME_CHOOSER)).andReturn(MockChooser1.class.getName()); + replay(confFactory, tableConf, systemConf); - thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForTable(); + VolumeChooser delegate = getTableDelegate(); + assertSame(MockChooser1.class, delegate.getClass()); } @Test - public void testTableMisconfiguredAndDefaultEmpty() throws Exception { - expectDefaultScope(""); - expectTableChooser(INVALID_CHOOSER_CLASSNAME); + public void testTableScopeUsingDefaultScopeProperty() throws Exception { + expect(tableConf.get(PerTableVolumeChooser.TABLE_VOLUME_CHOOSER)).andReturn(null).once(); + expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn(MockChooser2.class.getName()).once(); + replay(confFactory, tableConf, systemConf); - EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration); - - thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForTable(); + VolumeChooser delegate = getTableDelegate(); + assertSame(MockChooser2.class, delegate.getClass()); } @Test - public void testTableEmptyConfig() throws Exception { - expectDefaultScope(""); - expectTableChooser(""); - - EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration); + public void testTableScopeWithNoConfig() throws Exception { + expect(tableConf.get(PerTableVolumeChooser.TABLE_VOLUME_CHOOSER)).andReturn(null).once(); + expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn(null).once(); + replay(confFactory, tableConf, systemConf); thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForTable(); + getTableDelegate(); + fail("should not reach"); } @Test - public void testTableAndDefaultEmpty() throws Exception { - expectDefaultScope(""); - expectTableChooser(""); - - EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration); + public void testTableScopeWithBadDelegate() throws Exception { + expect(tableConf.get(PerTableVolumeChooser.TABLE_VOLUME_CHOOSER)).andReturn(null).once(); + expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn("not a valid class name").once(); + replay(confFactory, tableConf, systemConf); thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForTable(); + getTableDelegate(); + fail("should not reach"); } @Test - public void testScopeConfig() throws Exception { - expectLoggerScope(VolumeChooserAlwaysOne.class.getName()).atLeastOnce(); - - EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration); + public void testLoggerScopeUsingLoggerProperty() throws Exception { + expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(MockChooser1.class.getName()).once(); + replay(confFactory, tableConf, systemConf); - Set results = chooseRepeatedlyForLogger(); - - EasyMock.verify(mockedServerConfigurationFactory, mockedAccumuloConfiguration); - Assert.assertEquals(Sets.newHashSet(Arrays.asList("1")), results); + VolumeChooser delegate = getDelegate(ChooserScope.LOGGER); + assertSame(MockChooser1.class, delegate.getClass()); } @Test - public void testScopeMisconfigured() throws Exception { - expectDefaultScope(VolumeChooserAlwaysThree.class.getName()); - expectLoggerScope(INVALID_CHOOSER_CLASSNAME); - - EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration); + public void testLoggerScopeUsingDefaultProperty() throws Exception { + expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(null).once(); + expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn(MockChooser2.class.getName()).once(); + replay(confFactory, tableConf, systemConf); - thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForLogger(); + VolumeChooser delegate = getDelegate(ChooserScope.LOGGER); + assertSame(MockChooser2.class, delegate.getClass()); } @Test - public void testScopeMissing() throws Exception { - expectLoggerScope(null); - expectDefaultScope(null); - - EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration); + public void testLoggerScopeWithNoConfig() throws Exception { + expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(null).once(); + expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn(null).once(); + replay(confFactory, tableConf, systemConf); thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForLogger(); + getDelegate(ChooserScope.LOGGER); + fail("should not reach"); } @Test - public void testScopeMisconfiguredAndDefaultEmpty() throws Exception { - expectDefaultScope(""); - expectTableChooser(""); - expectLoggerScope(INVALID_CHOOSER_CLASSNAME); - - EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration); - - thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForLogger(); - } - - @Test - public void testScopeAndDefaultBothEmpty() throws Exception { - expectDefaultScope(""); - expectLoggerScope(""); - - EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration); + public void testLoggerScopeWithBadDelegate() throws Exception { + expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(null).once(); + expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn("not a valid class name").once(); + replay(confFactory, tableConf, systemConf); thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForLogger(); + getDelegate(ChooserScope.LOGGER); + fail("should not reach"); } } diff --git a/server/base/src/test/java/org/apache/accumulo/server/fs/PreferredVolumeChooserTest.java b/server/base/src/test/java/org/apache/accumulo/server/fs/PreferredVolumeChooserTest.java index 440c628..2110b40 100644 --- a/server/base/src/test/java/org/apache/accumulo/server/fs/PreferredVolumeChooserTest.java +++ b/server/base/src/test/java/org/apache/accumulo/server/fs/PreferredVolumeChooserTest.java @@ -16,10 +16,16 @@ */ package org.apache.accumulo.server.fs; -import java.lang.reflect.Field; +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.createStrictMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.fail; + import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; import org.apache.accumulo.core.client.impl.Table; import org.apache.accumulo.core.conf.AccumuloConfiguration; @@ -27,224 +33,167 @@ import org.apache.accumulo.server.conf.ServerConfigurationFactory; import org.apache.accumulo.server.conf.TableConfiguration; import org.apache.accumulo.server.fs.VolumeChooser.VolumeChooserException; import org.apache.accumulo.server.fs.VolumeChooserEnvironment.ChooserScope; -import org.easymock.EasyMock; -import org.easymock.IExpectationSetters; -import org.junit.Assert; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import com.google.common.collect.Sets; - public class PreferredVolumeChooserTest { - private static final int REQUIRED_NUMBER_TRIES = 20; // times to call choose for likely exercising of each preferred volume + private static final String[] ALL_OPTIONS = new String[] {"1", "2", "3"}; - private ServerConfigurationFactory mockedServerConfigurationFactory; - private TableConfiguration mockedTableConfiguration; - private PreferredVolumeChooser preferredVolumeChooser; - private AccumuloConfiguration mockedAccumuloConfiguration; + + private ServerConfigurationFactory confFactory; + private TableConfiguration tableConf; + private PreferredVolumeChooser chooser; + private AccumuloConfiguration systemConf; @Rule public ExpectedException thrown = ExpectedException.none(); @Before public void before() throws Exception { - preferredVolumeChooser = new PreferredVolumeChooser(); - - mockedServerConfigurationFactory = EasyMock.createMock(ServerConfigurationFactory.class); - Field field = preferredVolumeChooser.getClass().getDeclaredField("lazyConfFactory"); - field.setAccessible(true); - field.set(preferredVolumeChooser, mockedServerConfigurationFactory); + confFactory = createStrictMock(ServerConfigurationFactory.class); - mockedTableConfiguration = EasyMock.createMock(TableConfiguration.class); - mockedAccumuloConfiguration = EasyMock.createMock(AccumuloConfiguration.class); - EasyMock.expect(mockedServerConfigurationFactory.getTableConfiguration(EasyMock. anyObject())).andReturn(mockedTableConfiguration).anyTimes(); - EasyMock.expect(mockedServerConfigurationFactory.getSystemConfiguration()).andReturn(mockedAccumuloConfiguration).anyTimes(); - } - - private IExpectationSetters expectTableVolumes(String configuredVolumes) { - return EasyMock.expect(mockedTableConfiguration.get(PreferredVolumeChooser.TABLE_PREFERRED_VOLUMES)).andReturn(configuredVolumes); - } - - private IExpectationSetters expectDefaultScope(String configuredVolumes) { - return expectScope(ChooserScope.DEFAULT, configuredVolumes); - } + chooser = new PreferredVolumeChooser() { + @Override + ServerConfigurationFactory loadConfFactory() { + return confFactory; + } + }; - private IExpectationSetters expectLoggerScope(String configuredVolumes) { - return expectScope(ChooserScope.LOGGER, configuredVolumes); + tableConf = createStrictMock(TableConfiguration.class); + systemConf = createStrictMock(AccumuloConfiguration.class); + expect(confFactory.getTableConfiguration(anyObject())).andReturn(tableConf).anyTimes(); + expect(confFactory.getSystemConfiguration()).andReturn(systemConf).anyTimes(); } - private IExpectationSetters expectScope(ChooserScope scope, String configuredVolumes) { - return EasyMock.expect(mockedAccumuloConfiguration.get(PreferredVolumeChooser.getPropertyNameForScope(scope))).andReturn(configuredVolumes); + @After + public void after() throws Exception { + verify(confFactory, tableConf, systemConf); } - private Set chooseRepeatedlyForTable() { - VolumeChooserEnvironment volumeChooserEnvironment = new VolumeChooserEnvironment(Table.ID.of("h")); - Set results = new HashSet<>(); - for (int i = 0; i < REQUIRED_NUMBER_TRIES; i++) { - results.add(preferredVolumeChooser.choose(volumeChooserEnvironment, ALL_OPTIONS)); - } - return results; + private String[] chooseForTable() { + VolumeChooserEnvironment env = new VolumeChooserEnvironment(Table.ID.of("testTable")); + return chooser.getPreferredVolumes(env, ALL_OPTIONS); } - private Set chooseRepeatedlyForLogger() { - return chooseRepeatedlyForScope(ChooserScope.LOGGER); - } - - private Set chooseRepeatedlyForScope(ChooserScope scope) { - VolumeChooserEnvironment volumeChooserEnvironment = new VolumeChooserEnvironment(scope); - Set results = new HashSet<>(); - - for (int i = 0; i < REQUIRED_NUMBER_TRIES; i++) { - results.add(preferredVolumeChooser.choose(volumeChooserEnvironment, ALL_OPTIONS)); - } - return results; + private String[] choose(ChooserScope scope) { + VolumeChooserEnvironment env = new VolumeChooserEnvironment(scope); + return chooser.getPreferredVolumes(env, ALL_OPTIONS); } @Test - public void testEmptyEnvUsesRandomChooser() throws Exception { - VolumeChooserEnvironment volumeChooserEnvironment = new VolumeChooserEnvironment(ChooserScope.INIT); - Set results = new HashSet<>(); - for (int i = 0; i < REQUIRED_NUMBER_TRIES; i++) { - results.add(preferredVolumeChooser.choose(volumeChooserEnvironment, ALL_OPTIONS)); - } - - Assert.assertEquals(Sets.newHashSet(Arrays.asList(ALL_OPTIONS)), results); - } - - @Test - public void testTableConfig() throws Exception { - expectDefaultScope(null).anyTimes(); - expectTableVolumes("1,2").atLeastOnce(); - - EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration); - - Set results = chooseRepeatedlyForTable(); - - EasyMock.verify(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration); - Assert.assertEquals(Sets.newHashSet(Arrays.asList("1", "2")), results); + public void testInitScopeSelectsRandomlyFromAll() throws Exception { + replay(confFactory, tableConf, systemConf); + String[] volumes = choose(ChooserScope.INIT); + assertSame(ALL_OPTIONS, volumes); } @Test - public void testTableMisconfigured() throws Exception { - expectDefaultScope("1,3"); - expectTableVolumes("4"); + public void testTableScopeUsingTableProperty() throws Exception { + expect(tableConf.get(PreferredVolumeChooser.TABLE_PREFERRED_VOLUMES)).andReturn("2,1"); + replay(confFactory, tableConf, systemConf); - EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration); - - thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForTable(); + String[] volumes = chooseForTable(); + Arrays.sort(volumes); + assertArrayEquals(new String[] {"1", "2"}, volumes); } @Test - public void testTableMissing() throws Exception { - expectDefaultScope(""); - expectTableVolumes(null); - - EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration); + public void testTableScopeUsingDefaultScopeProperty() throws Exception { + expect(tableConf.get(PreferredVolumeChooser.TABLE_PREFERRED_VOLUMES)).andReturn(null).once(); + expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn("3,2").once(); + replay(confFactory, tableConf, systemConf); - thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForTable(); + String[] volumes = chooseForTable(); + Arrays.sort(volumes); + assertArrayEquals(new String[] {"2", "3"}, volumes); } @Test - public void testTableEmptyConfig() throws Exception { - expectDefaultScope(null); - expectTableVolumes(""); - - EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration); + public void testTableScopeWithNoConfig() throws Exception { + expect(tableConf.get(PreferredVolumeChooser.TABLE_PREFERRED_VOLUMES)).andReturn(null).once(); + expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn(null).once(); + replay(confFactory, tableConf, systemConf); thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForTable(); + chooseForTable(); + fail("should not reach"); } @Test - public void testTableMisconfiguredAndDefaultEmpty() throws Exception { - expectDefaultScope(""); - expectTableVolumes("4"); - - EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration); + public void testTableScopeWithEmptySet() throws Exception { + expect(tableConf.get(PreferredVolumeChooser.TABLE_PREFERRED_VOLUMES)).andReturn(",").once(); + replay(confFactory, tableConf, systemConf); thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForTable(); + chooseForTable(); + fail("should not reach"); } @Test - public void testTableAndDefaultEmpty() throws Exception { - expectDefaultScope(""); - expectTableVolumes(""); - - EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration); + public void testTableScopeWithUnrecognizedVolumes() throws Exception { + expect(tableConf.get(PreferredVolumeChooser.TABLE_PREFERRED_VOLUMES)).andReturn(null).once(); + expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn("4").once(); + replay(confFactory, tableConf, systemConf); thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForTable(); + chooseForTable(); + fail("should not reach"); } @Test - public void testScopeConfig() throws Exception { - expectLoggerScope("1,2").atLeastOnce(); - - EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration); + public void testLoggerScopeUsingLoggerProperty() throws Exception { + expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn("2,1").once(); + replay(confFactory, tableConf, systemConf); - Set results = chooseRepeatedlyForLogger(); - - EasyMock.verify(mockedServerConfigurationFactory, mockedAccumuloConfiguration); - Assert.assertEquals(Sets.newHashSet(Arrays.asList("1", "2")), results); + String[] volumes = choose(ChooserScope.LOGGER); + Arrays.sort(volumes); + assertArrayEquals(new String[] {"1", "2"}, volumes); } @Test - public void testScopeMisconfigured() throws Exception { - expectDefaultScope("1,3"); - expectLoggerScope("4"); - - EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration); + public void testLoggerScopeUsingDefaultProperty() throws Exception { + expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(null).once(); + expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn("3,2").once(); + replay(confFactory, tableConf, systemConf); - thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForLogger(); + String[] volumes = choose(ChooserScope.LOGGER); + Arrays.sort(volumes); + assertArrayEquals(new String[] {"2", "3"}, volumes); } @Test - public void testScopeMissing() throws Exception { - expectDefaultScope("").atLeastOnce(); - expectLoggerScope(null).atLeastOnce(); - - EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration); + public void testLoggerScopeWithNoConfig() throws Exception { + expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(null).once(); + expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn(null).once(); + replay(confFactory, tableConf, systemConf); thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForLogger(); + choose(ChooserScope.LOGGER); + fail("should not reach"); } @Test - public void testScopeMisconfiguredAndDefaultEmpty() throws Exception { - expectDefaultScope(""); - expectLoggerScope("4"); - - EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration); + public void testLoggerScopeWithEmptySet() throws Exception { + expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(",").once(); + replay(confFactory, tableConf, systemConf); thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForLogger(); + choose(ChooserScope.LOGGER); + fail("should not reach"); } @Test - public void testScopeAndDefaultBothEmpty() throws Exception { - expectDefaultScope(""); - expectLoggerScope(""); - - EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration); + public void testLoggerScopeWithUnrecognizedVolumes() throws Exception { + expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(null).once(); + expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn("4").once(); + replay(confFactory, tableConf, systemConf); thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForLogger(); + choose(ChooserScope.LOGGER); + fail("should not reach"); } - @Test - public void testScopeEmptyConfig() throws Exception { - expectDefaultScope(""); - expectLoggerScope(""); - - EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration); - - thrown.expect(VolumeChooserException.class); - chooseRepeatedlyForLogger(); - } } -- To stop receiving notification emails like this one, please contact "commits@accumulo.apache.org" .