Merge pull request #2799 from b-slim/fix_snapshot

MapLookupFactory need to be Ser/Desr ready.

processing/src/main/java/io/druid/query/extraction/MapLookupExtractorFactory.java

@@ -30,7 +30,9 @@ import java.util.Map;
 
 public class MapLookupExtractorFactory implements LookupExtractorFactory
 {
+  @JsonProperty
   private final Map<String, String> map;
+  @JsonProperty
   private final boolean isOneToOne;
   private final MapLookupExtractor lookupExtractor;
 

processing/src/test/java/io/druid/query/dimension/LookupDimensionSpecTest.java

@@ -25,10 +25,10 @@ import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableMap;
 import io.druid.jackson.DefaultObjectMapper;
 import io.druid.query.extraction.ExtractionFn;
-import io.druid.query.lookup.LookupExtractor;
-import io.druid.query.lookup.LookupExtractorFactory;
-import io.druid.query.lookup.LookupReferencesManager;
 import io.druid.query.extraction.MapLookupExtractor;
+import io.druid.query.extraction.MapLookupExtractorFactory;
+import io.druid.query.lookup.LookupExtractor;
+import io.druid.query.lookup.LookupReferencesManager;
 import junitparams.JUnitParamsRunner;
 import junitparams.Parameters;
 import org.easymock.EasyMock;
@@ -36,7 +36,6 @@ import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
-import javax.annotation.Nullable;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Map;
@@ -51,32 +50,8 @@ public class LookupDimensionSpecTest
   private static final LookupReferencesManager LOOKUP_REF_MANAGER = EasyMock.createMock(LookupReferencesManager.class);
 
   static {
-    EasyMock.expect(LOOKUP_REF_MANAGER.get(EasyMock.eq("lookupName"))).andReturn(new LookupExtractorFactory()
+    EasyMock.expect(LOOKUP_REF_MANAGER.get(EasyMock.eq("lookupName"))).andReturn(new MapLookupExtractorFactory(STRING_MAP, false)
-    {
+    ).anyTimes();
-      @Override
-      public boolean start()
-      {
-        return true;
-      }
-
-      @Override
-      public boolean close()
-      {
-        return true;
-      }
-
-      @Override
-      public boolean replaces(@Nullable LookupExtractorFactory other)
-      {
-        return true;
-      }
-
-      @Override
-      public LookupExtractor get()
-      {
-        return MAP_LOOKUP_EXTRACTOR;
-      }
-    }).anyTimes();
     EasyMock.replay(LOOKUP_REF_MANAGER);
   }
 

processing/src/test/java/io/druid/query/extraction/MapLookupExtractorFactoryTest.java

@@ -19,10 +19,15 @@
 
 package io.druid.query.extraction;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.collect.ImmutableMap;
+import io.druid.jackson.DefaultObjectMapper;
+import io.druid.query.lookup.LookupExtractorFactory;
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.io.IOException;
+
 public class MapLookupExtractorFactoryTest
 {
   private static final String KEY = "foo";
@@ -46,4 +51,12 @@ public class MapLookupExtractorFactoryTest
     Assert.assertTrue(factory.replaces(new MapLookupExtractorFactory(ImmutableMap.of(KEY, VALUE + "1"), true)));
     Assert.assertTrue(factory.replaces(null));
   }
+
+  @Test
+  public void testSerDeserMapLookupExtractorFactory() throws IOException
+  {
+    ObjectMapper mapper = new DefaultObjectMapper();
+    LookupExtractorFactory lookupExtractorFactory = new MapLookupExtractorFactory(ImmutableMap.of("key", "value"), true);
+    Assert.assertEquals(lookupExtractorFactory, mapper.reader(LookupExtractorFactory.class).readValue(mapper.writeValueAsString(lookupExtractorFactory)));
+  }
 }

processing/src/test/java/io/druid/query/lookup/LookupReferencesManagerTest.java

@@ -19,14 +19,12 @@
 
 package io.druid.query.lookup;
 
-import com.fasterxml.jackson.annotation.JsonCreator;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.annotation.JsonTypeName;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.io.Files;
 import com.metamx.common.ISE;
 import io.druid.jackson.DefaultObjectMapper;
+import io.druid.query.extraction.MapLookupExtractorFactory;
 import org.easymock.EasyMock;
 import org.junit.After;
 import org.junit.Assert;
@@ -35,7 +33,6 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
-import javax.annotation.Nullable;
 import java.io.IOException;
 
 public class LookupReferencesManagerTest
@@ -52,7 +49,6 @@ public class LookupReferencesManagerTest
     Assert.assertTrue("must be closed before start call", lookupReferencesManager.isClosed());
     lookupReferencesManager.start();
     Assert.assertFalse("must start after start call", lookupReferencesManager.isClosed());
-    mapper.registerSubtypes(LookupExtractorFactoryMock.class);
   }
 
   @After
@@ -257,7 +253,7 @@ public class LookupReferencesManagerTest
   @Test
   public void testBootstrapFromFile() throws IOException
   {
-    LookupExtractorFactory lookupExtractorFactory = new LookupExtractorFactoryMock("data");
+    LookupExtractorFactory lookupExtractorFactory = new MapLookupExtractorFactory(ImmutableMap.<String, String>of("key", "value"), true);
     lookupReferencesManager.put("testMockForBootstrap",lookupExtractorFactory);
     lookupReferencesManager.stop();
     lookupReferencesManager.start();
@@ -265,68 +261,4 @@ public class LookupReferencesManagerTest
 
   }
 
-  @JsonTypeName("mockTest")
-  private static class LookupExtractorFactoryMock implements LookupExtractorFactory
-  {
-    @JsonProperty
-    public String getData()
-    {
-      return dataString;
-    }
-
-    @JsonProperty
-    private final String dataString;
-
-    @JsonCreator
-    public LookupExtractorFactoryMock(@JsonProperty("dataString") String dataString)
-    {
-      this.dataString = dataString;
-    }
-
-    @Override
-    public boolean start()
-    {
-      return true;
-    }
-
-    @Override
-    public boolean close()
-    {
-      return true;
-    }
-
-    @Override
-    public boolean replaces(@Nullable LookupExtractorFactory other)
-    {
-      return false;
-    }
-
-    @Override
-    public LookupExtractor get()
-    {
-      return null;
-    }
-
-    @Override
-    public int hashCode()
-    {
-      return dataString != null ? dataString.hashCode() : 0;
-    }
-
-    @Override
-    public boolean equals(Object o)
-    {
-      if (this == o) {
-        return true;
-      }
-      if (!(o instanceof LookupExtractorFactoryMock)) {
-        return false;
-      }
-
-      LookupExtractorFactoryMock that = (LookupExtractorFactoryMock) o;
-
-      return getData().equals(that.getData());
-
-    }
-  }
 }

processing/src/test/java/io/druid/query/lookup/LookupSnapshotTakerTest.java

@@ -19,13 +19,13 @@
 
 package io.druid.query.lookup;
 
-import com.fasterxml.jackson.annotation.JsonCreator;
-import com.fasterxml.jackson.annotation.JsonTypeName;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import com.google.common.io.Files;
 import com.metamx.common.ISE;
 import com.metamx.common.StringUtils;
+import io.druid.query.extraction.MapLookupExtractorFactory;
 import io.druid.segment.TestHelper;
 import org.junit.Assert;
 import org.junit.Before;
@@ -33,7 +33,6 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
-import javax.annotation.Nullable;
 import java.io.File;
 import java.io.IOException;
 import java.util.Collections;
@@ -55,9 +54,6 @@ public class LookupSnapshotTakerTest
   {
     basePersistDirectory = temporaryFolder.newFolder().getAbsolutePath();
     lookupSnapshotTaker = new LookupSnapshotTaker(mapper, basePersistDirectory);
-    mapper.registerSubtypes(LookupFactoryMock.class);
-    mapper.registerSubtypes(LookupExtractorFactory.class);
-    mapper.addMixIn(LookupExtractorFactory.class, LookupFactoryMock.class);
   }
 
   @Test
@@ -66,7 +62,7 @@ public class LookupSnapshotTakerTest
 
     LookupBean lookupBean = new LookupBean();
     lookupBean.name = "name";
-    lookupBean.factory = new LookupFactoryMock();
+    lookupBean.factory = new MapLookupExtractorFactory(ImmutableMap.of("key", "value"), true);
     List<LookupBean> lookupBeanList = Lists.newArrayList(lookupBean);
     lookupSnapshotTaker.takeSnapshot(lookupBeanList);
     List<LookupBean> actualList = lookupSnapshotTaker.pullExistingSnapshot();
@@ -81,7 +77,7 @@ public class LookupSnapshotTakerTest
     LookupSnapshotTaker lookupSnapshotTaker = new LookupSnapshotTaker(mapper, directory.getAbsolutePath());
     LookupBean lookupBean = new LookupBean();
     lookupBean.name = "name";
-    lookupBean.factory = new LookupFactoryMock();
+    lookupBean.factory = new MapLookupExtractorFactory(ImmutableMap.of("key", "value"), true);
     List<LookupBean> lookupBeanList = Lists.newArrayList(lookupBean);
     lookupSnapshotTaker.takeSnapshot(lookupBeanList);
   }
@@ -113,44 +109,4 @@ public class LookupSnapshotTakerTest
     List<LookupBean> actualList = lookupSnapshotTaker.pullExistingSnapshot();
     Assert.assertEquals(Collections.EMPTY_LIST, actualList);
   }
-
-  @JsonTypeName("mock")
-  private class LookupFactoryMock implements LookupExtractorFactory
-  {
-
-    @JsonCreator
-    public LookupFactoryMock()
-    {
-    }
-
-    @Override
-    public boolean start()
-    {
-      return true;
-    }
-
-    @Override
-    public boolean close()
-    {
-      return true;
-    }
-
-    @Override
-    public boolean replaces(@Nullable LookupExtractorFactory other)
-    {
-      return false;
-    }
-
-    @Override
-    public LookupExtractor get()
-    {
-      return null;
-    }
-
-    @Override
-    public boolean equals(Object obj)
-    {
-      return obj instanceof LookupFactoryMock;
-    }
-  }
 }