From 5fc68914046c9fcc16a26e15fe798f4c9ebb330a Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Wed, 18 Oct 2017 13:57:49 -0700 Subject: [PATCH] Reduce code duplication between test ExprMacroTables. (#4979) --- .../io/druid/math/expr/ExprMacroTable.java | 6 ++++ .../io/druid/indexing/common/TestUtils.java | 4 +-- .../druid/query/expression/ExprMacroTest.java | 2 +- ...a => LookupEnabledTestExprMacroTable.java} | 34 +++++++++---------- .../druid/sql/calcite/util/CalciteTests.java | 4 +-- 5 files changed, 28 insertions(+), 22 deletions(-) rename server/src/test/java/io/druid/query/expression/{TestExpressionMacroTable.java => LookupEnabledTestExprMacroTable.java} (78%) diff --git a/common/src/main/java/io/druid/math/expr/ExprMacroTable.java b/common/src/main/java/io/druid/math/expr/ExprMacroTable.java index 5b9a33c7e87..8c0240aed09 100644 --- a/common/src/main/java/io/druid/math/expr/ExprMacroTable.java +++ b/common/src/main/java/io/druid/math/expr/ExprMacroTable.java @@ -19,6 +19,7 @@ package io.druid.math.expr; +import com.google.common.collect.ImmutableList; import io.druid.java.util.common.StringUtils; import javax.annotation.Nullable; @@ -48,6 +49,11 @@ public class ExprMacroTable return NIL; } + public List getMacros() + { + return ImmutableList.copyOf(macroMap.values()); + } + /** * Returns an expr corresponding to a function call if this table has an entry for {@code functionName}. * Otherwise, returns null. diff --git a/indexing-service/src/test/java/io/druid/indexing/common/TestUtils.java b/indexing-service/src/test/java/io/druid/indexing/common/TestUtils.java index e34ea20fbc2..c7bbb95471b 100644 --- a/indexing-service/src/test/java/io/druid/indexing/common/TestUtils.java +++ b/indexing-service/src/test/java/io/druid/indexing/common/TestUtils.java @@ -29,7 +29,7 @@ import io.druid.jackson.DefaultObjectMapper; import io.druid.java.util.common.ISE; import io.druid.java.util.common.logger.Logger; import io.druid.math.expr.ExprMacroTable; -import io.druid.query.expression.TestExpressionMacroTable; +import io.druid.query.expression.LookupEnabledTestExprMacroTable; import io.druid.segment.IndexIO; import io.druid.segment.IndexMergerV9; import io.druid.segment.column.ColumnConfig; @@ -74,7 +74,7 @@ public class TestUtils jsonMapper.setInjectableValues( new InjectableValues.Std() - .addValue(ExprMacroTable.class.getName(), TestExpressionMacroTable.INSTANCE) + .addValue(ExprMacroTable.class.getName(), LookupEnabledTestExprMacroTable.INSTANCE) .addValue(IndexIO.class, indexIO) .addValue(ObjectMapper.class, jsonMapper) .addValue(ChatHandlerProvider.class, new NoopChatHandlerProvider()) diff --git a/server/src/test/java/io/druid/query/expression/ExprMacroTest.java b/server/src/test/java/io/druid/query/expression/ExprMacroTest.java index 90353a16801..e7bb2397fe2 100644 --- a/server/src/test/java/io/druid/query/expression/ExprMacroTest.java +++ b/server/src/test/java/io/druid/query/expression/ExprMacroTest.java @@ -174,7 +174,7 @@ public class ExprMacroTest private void assertExpr(final String expression, final Object expectedResult) { - final Expr expr = Parser.parse(expression, TestExpressionMacroTable.INSTANCE); + final Expr expr = Parser.parse(expression, LookupEnabledTestExprMacroTable.INSTANCE); Assert.assertEquals(expression, expectedResult, expr.eval(BINDINGS).value()); } } diff --git a/server/src/test/java/io/druid/query/expression/TestExpressionMacroTable.java b/server/src/test/java/io/druid/query/expression/LookupEnabledTestExprMacroTable.java similarity index 78% rename from server/src/test/java/io/druid/query/expression/TestExpressionMacroTable.java rename to server/src/test/java/io/druid/query/expression/LookupEnabledTestExprMacroTable.java index ecc5fde256a..b15feb05aeb 100644 --- a/server/src/test/java/io/druid/query/expression/TestExpressionMacroTable.java +++ b/server/src/test/java/io/druid/query/expression/LookupEnabledTestExprMacroTable.java @@ -19,8 +19,9 @@ package io.druid.query.expression; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; import io.druid.math.expr.ExprMacroTable; import io.druid.query.extraction.MapLookupExtractor; import io.druid.query.lookup.LookupExtractor; @@ -31,27 +32,26 @@ import io.druid.query.lookup.LookupReferencesManager; import org.easymock.EasyMock; import javax.annotation.Nullable; +import java.util.Collections; -public class TestExpressionMacroTable extends ExprMacroTable +/** + * Separate from {@link TestExprMacroTable} since that one is in druid-processing, which doesn't have + * {@link LookupReferencesManager}. + */ +public class LookupEnabledTestExprMacroTable extends ExprMacroTable { - public static final ExprMacroTable INSTANCE = new TestExpressionMacroTable(); + public static final ExprMacroTable INSTANCE = new LookupEnabledTestExprMacroTable(); - private TestExpressionMacroTable() + private LookupEnabledTestExprMacroTable() { super( - ImmutableList.of( - new LikeExprMacro(), - new LookupExprMacro(createTestLookupReferencesManager(ImmutableMap.of("foo", "xfoo"))), - new RegexpExtractExprMacro(), - new TimestampCeilExprMacro(), - new TimestampExtractExprMacro(), - new TimestampFloorExprMacro(), - new TimestampFormatExprMacro(), - new TimestampParseExprMacro(), - new TimestampShiftExprMacro(), - new TrimExprMacro.BothTrimExprMacro(), - new TrimExprMacro.LeftTrimExprMacro(), - new TrimExprMacro.RightTrimExprMacro() + Lists.newArrayList( + Iterables.concat( + TestExprMacroTable.INSTANCE.getMacros(), + Collections.singletonList( + new LookupExprMacro(createTestLookupReferencesManager(ImmutableMap.of("foo", "xfoo"))) + ) + ) ) ); } diff --git a/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java b/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java index 57902d75deb..c10e7619abd 100644 --- a/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java +++ b/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java @@ -58,7 +58,7 @@ import io.druid.query.aggregation.DoubleSumAggregatorFactory; import io.druid.query.aggregation.FloatSumAggregatorFactory; import io.druid.query.aggregation.hyperloglog.HyperUniquesAggregatorFactory; import io.druid.query.expression.LookupExprMacro; -import io.druid.query.expression.TestExpressionMacroTable; +import io.druid.query.expression.LookupEnabledTestExprMacroTable; import io.druid.query.groupby.GroupByQuery; import io.druid.query.groupby.GroupByQueryConfig; import io.druid.query.groupby.GroupByQueryRunnerTest; @@ -140,7 +140,7 @@ public class CalciteTests binder.bind(LookupReferencesManager.class) .toInstance( - TestExpressionMacroTable.createTestLookupReferencesManager( + LookupEnabledTestExprMacroTable.createTestLookupReferencesManager( ImmutableMap.of( "a", "xa", "abc", "xabc"