From 41d1631cb7b91b63b7a8c5d54c6e5f0440449600 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Tue, 16 Jun 2020 09:41:48 -0700 Subject: [PATCH 1/2] Remove LegacyDataSource. Its purpose was to enable deserialization of strings into TableDataSources. But we can do this more straightforwardly with Jackson annotations. --- .../org/apache/druid/query/DataSource.java | 2 +- .../apache/druid/query/LegacyDataSource.java | 33 ----------------- .../apache/druid/query/TableDataSource.java | 6 +++ .../druid/query/TableDataSourceTest.java | 37 ++++++++++++++----- .../query/metadata/SegmentAnalyzerTest.java | 4 +- .../server/log/FilteredRequestLoggerTest.java | 4 +- .../server/log/LoggingRequestLoggerTest.java | 6 +-- 7 files changed, 42 insertions(+), 50 deletions(-) delete mode 100644 processing/src/main/java/org/apache/druid/query/LegacyDataSource.java diff --git a/processing/src/main/java/org/apache/druid/query/DataSource.java b/processing/src/main/java/org/apache/druid/query/DataSource.java index 549b06b0b3eb..b78a45fc36c6 100644 --- a/processing/src/main/java/org/apache/druid/query/DataSource.java +++ b/processing/src/main/java/org/apache/druid/query/DataSource.java @@ -28,7 +28,7 @@ /** * Represents a source... of data... for a query. Analogous to the "FROM" clause in SQL. */ -@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type", defaultImpl = LegacyDataSource.class) +@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type", defaultImpl = TableDataSource.class) @JsonSubTypes({ @JsonSubTypes.Type(value = TableDataSource.class, name = "table"), @JsonSubTypes.Type(value = QueryDataSource.class, name = "query"), diff --git a/processing/src/main/java/org/apache/druid/query/LegacyDataSource.java b/processing/src/main/java/org/apache/druid/query/LegacyDataSource.java deleted file mode 100644 index d3812489be20..000000000000 --- a/processing/src/main/java/org/apache/druid/query/LegacyDataSource.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * 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.druid.query; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonTypeName; - -@JsonTypeName("table") -public class LegacyDataSource extends TableDataSource -{ - @JsonCreator - public LegacyDataSource(String name) - { - super(name); - } -} diff --git a/processing/src/main/java/org/apache/druid/query/TableDataSource.java b/processing/src/main/java/org/apache/druid/query/TableDataSource.java index 4c371cf84510..f9a07a181ee4 100644 --- a/processing/src/main/java/org/apache/druid/query/TableDataSource.java +++ b/processing/src/main/java/org/apache/druid/query/TableDataSource.java @@ -40,6 +40,12 @@ public TableDataSource(@JsonProperty("name") String name) this.name = Preconditions.checkNotNull(name, "'name' must be nonnull"); } + @JsonCreator + public static TableDataSource create(final String name) + { + return new TableDataSource(name); + } + @JsonProperty public String getName() { diff --git a/processing/src/test/java/org/apache/druid/query/TableDataSourceTest.java b/processing/src/test/java/org/apache/druid/query/TableDataSourceTest.java index ef50f3e45d9d..bf086670cf1f 100644 --- a/processing/src/test/java/org/apache/druid/query/TableDataSourceTest.java +++ b/processing/src/test/java/org/apache/druid/query/TableDataSourceTest.java @@ -89,27 +89,46 @@ public void test_equals() } @Test - public void test_equals_legacy() + public void test_serde_roundTrip() throws Exception { - final LegacyDataSource legacyFoo = new LegacyDataSource("foo"); - final LegacyDataSource legacyBar = new LegacyDataSource("bar"); + final ObjectMapper jsonMapper = TestHelper.makeJsonMapper(); + final TableDataSource deserialized = (TableDataSource) jsonMapper.readValue( + jsonMapper.writeValueAsString(fooDataSource), + DataSource.class + ); - Assert.assertEquals(legacyFoo, fooDataSource); - Assert.assertEquals(fooDataSource, legacyFoo); + Assert.assertEquals(fooDataSource, deserialized); + } - Assert.assertNotEquals(legacyBar, fooDataSource); - Assert.assertNotEquals(fooDataSource, legacyBar); + @Test + public void test_deserialize_fromObject() throws Exception + { + final ObjectMapper jsonMapper = TestHelper.makeJsonMapper(); + final TableDataSource deserialized = (TableDataSource) jsonMapper.readValue( + "{\"type\":\"table\",\"name\":\"foo\"}", + DataSource.class + ); + + Assert.assertEquals(fooDataSource, deserialized); } @Test - public void test_serde() throws Exception + public void test_deserialize_fromString() throws Exception { final ObjectMapper jsonMapper = TestHelper.makeJsonMapper(); final TableDataSource deserialized = (TableDataSource) jsonMapper.readValue( - jsonMapper.writeValueAsString(fooDataSource), + "\"foo\"", DataSource.class ); Assert.assertEquals(fooDataSource, deserialized); } + + @Test + public void test_serialize() throws Exception + { + final ObjectMapper jsonMapper = TestHelper.makeJsonMapper(); + final String s = jsonMapper.writeValueAsString(fooDataSource); + Assert.assertEquals("{\"type\":\"table\",\"name\":\"foo\"}", s); + } } diff --git a/processing/src/test/java/org/apache/druid/query/metadata/SegmentAnalyzerTest.java b/processing/src/test/java/org/apache/druid/query/metadata/SegmentAnalyzerTest.java index 4f3a81dbb208..66df87904daa 100644 --- a/processing/src/test/java/org/apache/druid/query/metadata/SegmentAnalyzerTest.java +++ b/processing/src/test/java/org/apache/druid/query/metadata/SegmentAnalyzerTest.java @@ -20,11 +20,11 @@ package org.apache.druid.query.metadata; import org.apache.druid.data.input.impl.DimensionSchema; -import org.apache.druid.query.LegacyDataSource; import org.apache.druid.query.QueryPlus; import org.apache.druid.query.QueryRunner; import org.apache.druid.query.QueryRunnerFactory; import org.apache.druid.query.QueryRunnerTestHelper; +import org.apache.druid.query.TableDataSource; import org.apache.druid.query.metadata.metadata.ColumnAnalysis; import org.apache.druid.query.metadata.metadata.SegmentAnalysis; import org.apache.druid.query.metadata.metadata.SegmentMetadataQuery; @@ -190,7 +190,7 @@ private List getSegmentAnalysises(Segment index, EnumSet queryContext = ImmutableMap.of("foo", "bar"); final Query query = new FakeQuery( - new LegacyDataSource("datasource"), + new TableDataSource("datasource"), new QuerySegmentSpec() { @Override @@ -127,7 +127,7 @@ public QueryRunner lookup(Query query, QuerySegmentWalker walker) ); final Query unionQuery = new FakeQuery( - new UnionDataSource(ImmutableList.of(new LegacyDataSource("A"), new LegacyDataSource("B"))), + new UnionDataSource(ImmutableList.of(new TableDataSource("A"), new TableDataSource("B"))), new QuerySegmentSpec() { @Override From 17d414ba525f580c6c6f63f8c580e55e2dca7ff8 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Tue, 16 Jun 2020 09:46:10 -0700 Subject: [PATCH 2/2] Slight test improvement. --- .../test/java/org/apache/druid/query/TableDataSourceTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/processing/src/test/java/org/apache/druid/query/TableDataSourceTest.java b/processing/src/test/java/org/apache/druid/query/TableDataSourceTest.java index bf086670cf1f..ec8d27d29029 100644 --- a/processing/src/test/java/org/apache/druid/query/TableDataSourceTest.java +++ b/processing/src/test/java/org/apache/druid/query/TableDataSourceTest.java @@ -36,6 +36,7 @@ public class TableDataSourceTest public ExpectedException expectedException = ExpectedException.none(); private final TableDataSource fooDataSource = new TableDataSource("foo"); + private final TableDataSource barDataSource = new TableDataSource("bar"); @Test public void test_getTableNames() @@ -98,6 +99,7 @@ public void test_serde_roundTrip() throws Exception ); Assert.assertEquals(fooDataSource, deserialized); + Assert.assertNotEquals(barDataSource, deserialized); } @Test @@ -110,6 +112,7 @@ public void test_deserialize_fromObject() throws Exception ); Assert.assertEquals(fooDataSource, deserialized); + Assert.assertNotEquals(barDataSource, deserialized); } @Test @@ -122,6 +125,7 @@ public void test_deserialize_fromString() throws Exception ); Assert.assertEquals(fooDataSource, deserialized); + Assert.assertNotEquals(barDataSource, deserialized); } @Test