Lazy initialization for JavaScript functions

api/src/main/java/io/druid/data/input/impl/JavaScriptParseSpec.java

@@ -22,7 +22,7 @@
 import com.fasterxml.jackson.annotation.JacksonInject;
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
-import io.druid.java.util.common.ISE;
+import com.google.common.base.Preconditions;
 import io.druid.java.util.common.parsers.JavaScriptParser;
 import io.druid.java.util.common.parsers.Parser;
 import io.druid.js.JavaScriptConfig;
@@ -36,6 +36,9 @@ public class JavaScriptParseSpec extends ParseSpec
   private final String function;
   private final JavaScriptConfig config;
 
+  // This variable is lazily initialized to avoid unnecessary JavaScript compilation during JSON serde
+  private JavaScriptParser parser;
+
   @JsonCreator
   public JavaScriptParseSpec(
       @JsonProperty("timestampSpec") TimestampSpec timestampSpec,
@@ -64,11 +67,11 @@ public void verify(List<String> usedCols)
   @Override
   public Parser<String, Object> makeParser()
   {
-    if (!config.isEnabled()) {
-      throw new ISE("JavaScript is disabled");
-    }
-
-    return new JavaScriptParser(function);
+    // JavaScript configuration should be checked when it's actually used because someone might still want Druid
+    // nodes to be able to deserialize JavaScript-based objects even though JavaScript is disabled.
+    Preconditions.checkState(config.isEnabled(), "JavaScript is disabled");
+    parser = parser == null ? new JavaScriptParser(function) : parser;
+    return parser;
   }
 
   @Override

api/src/main/java/io/druid/data/input/impl/StringInputRowParser.java

@@ -22,6 +22,7 @@
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Charsets;
+import com.google.common.base.Preconditions;
 import io.druid.data.input.ByteBufferInputRowParser;
 import io.druid.data.input.InputRow;
 import io.druid.java.util.common.parsers.ParseException;
@@ -43,20 +44,19 @@ public class StringInputRowParser implements ByteBufferInputRowParser
 
   private final ParseSpec parseSpec;
   private final MapInputRowParser mapParser;
-  private final Parser<String, Object> parser;
   private final Charset charset;
 
-  private CharBuffer chars = null;
+  private Parser<String, Object> parser;
+  private CharBuffer chars;
 
   @JsonCreator
   public StringInputRowParser(
       @JsonProperty("parseSpec") ParseSpec parseSpec,
       @JsonProperty("encoding") String encoding
   )
   {
-    this.parseSpec = parseSpec;
+    this.parseSpec = Preconditions.checkNotNull(parseSpec, "parseSpec");
     this.mapParser = new MapInputRowParser(parseSpec);
-    this.parser = parseSpec.makeParser();
 
     if (encoding != null) {
       this.charset = Charset.forName(encoding);
@@ -124,8 +124,18 @@ private Map<String, Object> buildStringKeyMap(ByteBuffer input)
     return theMap;
   }
 
+  public void initializeParser()
+  {
+    if (parser == null) {
+      // parser should be created when it is really used to avoid unnecessary initialization of the underlying
+      // parseSpec.
+      parser = parseSpec.makeParser();
+    }
+  }
+
   public void startFileFromBeginning()
   {
+    initializeParser();
     parser.startFileFromBeginning();
   }
 
@@ -138,6 +148,7 @@ public InputRow parse(@Nullable String input)
   @Nullable
   private Map<String, Object> parseString(@Nullable String inputString)
   {
+    initializeParser();
     return parser.parse(inputString);
   }
 

api/src/test/java/io/druid/data/input/impl/StringInputRowParserTest.java

@@ -0,0 +1,85 @@
+/*
+ * Licensed to Metamarkets Group Inc. (Metamarkets) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. Metamarkets 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 io.druid.data.input.impl;
+
+import com.google.common.collect.ImmutableList;
+import io.druid.js.JavaScriptConfig;
+import org.hamcrest.CoreMatchers;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+public class StringInputRowParserTest
+{
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
+
+  @Test
+  public void testDisableJavaScript()
+  {
+    final JavaScriptParseSpec parseSpec = new JavaScriptParseSpec(
+        new TimestampSpec("timestamp", "auto", null),
+        new DimensionsSpec(
+            DimensionsSpec.getDefaultSchemas(
+                ImmutableList.of(
+                    "dim1",
+                    "dim2"
+                )
+            ),
+            null,
+            null
+        ),
+        "func",
+        new JavaScriptConfig(false)
+    );
+    final StringInputRowParser parser = new StringInputRowParser(parseSpec, "UTF-8");
+
+    expectedException.expect(CoreMatchers.instanceOf(IllegalStateException.class));
+    expectedException.expectMessage("JavaScript is disabled");
+
+    parser.startFileFromBeginning();
+  }
+
+  @Test
+  public void testDisableJavaScript2()
+  {
+    final JavaScriptParseSpec parseSpec = new JavaScriptParseSpec(
+        new TimestampSpec("timestamp", "auto", null),
+        new DimensionsSpec(
+            DimensionsSpec.getDefaultSchemas(
+                ImmutableList.of(
+                    "dim1",
+                    "dim2"
+                )
+            ),
+            null,
+            null
+        ),
+        "func",
+        new JavaScriptConfig(false)
+    );
+    final StringInputRowParser parser = new StringInputRowParser(parseSpec, "UTF-8");
+
+    expectedException.expect(CoreMatchers.instanceOf(IllegalStateException.class));
+    expectedException.expectMessage("JavaScript is disabled");
+
+    parser.parse("");
+  }
+}

extensions-contrib/thrift-extensions/src/main/java/io/druid/data/input/thrift/ThriftInputRowParser.java

@@ -52,7 +52,7 @@ public class ThriftInputRowParser implements InputRowParser<Object>
   private final String jarPath;
   private final String thriftClassName;
 
-  final private Parser<String, Object> parser;
+  private Parser<String, Object> parser;
   volatile private Class<TBase> thriftClass = null;
 
   @JsonCreator
@@ -67,7 +67,6 @@ public ThriftInputRowParser(
     Preconditions.checkNotNull(thriftClassName, "thrift class name");
 
     this.parseSpec = parseSpec;
-    parser = parseSpec.makeParser();
   }
 
   public Class<TBase> getThriftClass()
@@ -92,6 +91,12 @@ public Class<TBase> getThriftClass()
   @Override
   public InputRow parse(Object input)
   {
+    if (parser == null) {
+      // parser should be created when it is really used to avoid unnecessary initialization of the underlying
+      // parseSpec.
+      parser = parseSpec.makeParser();
+    }
+
     // There is a Parser check in phase 2 of mapreduce job, thrift jar may not present in peon side.
     // Place it this initialization in constructor will get ClassNotFoundException
     try {

extensions-contrib/thrift-extensions/src/test/java/io/druid/data/input/thrift/ThriftInputRowParserTest.java

@@ -19,26 +19,32 @@
 
 package io.druid.data.input.thrift;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import io.druid.data.input.InputRow;
 import io.druid.data.input.impl.DimensionSchema;
 import io.druid.data.input.impl.DimensionsSpec;
 import io.druid.data.input.impl.JSONParseSpec;
+import io.druid.data.input.impl.JavaScriptParseSpec;
 import io.druid.data.input.impl.ParseSpec;
 import io.druid.data.input.impl.StringDimensionSchema;
 import io.druid.data.input.impl.TimestampSpec;
 import io.druid.java.util.common.parsers.JSONPathFieldSpec;
 import io.druid.java.util.common.parsers.JSONPathFieldType;
 import io.druid.java.util.common.parsers.JSONPathSpec;
+import io.druid.js.JavaScriptConfig;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.hadoop.io.BytesWritable;
 import org.apache.thrift.TException;
 import org.apache.thrift.TSerializer;
 import org.apache.thrift.protocol.TBinaryProtocol;
 import org.apache.thrift.protocol.TCompactProtocol;
 import org.apache.thrift.protocol.TJSONProtocol;
+import org.hamcrest.CoreMatchers;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 import java.nio.ByteBuffer;
 
@@ -47,6 +53,8 @@
 
 public class ThriftInputRowParserTest
 {
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
 
   private ParseSpec parseSpec;
 
@@ -111,6 +119,36 @@ public void testParse() throws Exception
     serializationAndTest(parser, bytes);
   }
 
+  @Test
+  public void testDisableJavaScript()
+  {
+    final JavaScriptParseSpec parseSpec = new JavaScriptParseSpec(
+        new TimestampSpec("timestamp", "auto", null),
+        new DimensionsSpec(
+            DimensionsSpec.getDefaultSchemas(
+                ImmutableList.of(
+                    "dim1",
+                    "dim2"
+                )
+            ),
+            null,
+            null
+        ),
+        "func",
+        new JavaScriptConfig(false)
+    );
+    ThriftInputRowParser parser = new ThriftInputRowParser(
+        parseSpec,
+        "example/book.jar",
+        "io.druid.data.input.thrift.Book"
+    );
+
+    expectedException.expect(CoreMatchers.instanceOf(IllegalStateException.class));
+    expectedException.expectMessage("JavaScript is disabled");
+
+    parser.parse(ByteBuffer.allocate(1));
+  }
+
   public void serializationAndTest(ThriftInputRowParser parser, byte[] bytes) throws TException
   {
     ByteBuffer buffer = ByteBuffer.wrap(bytes);

extensions-core/kafka-indexing-service/src/test/java/io/druid/indexing/kafka/KafkaIndexTaskTest.java

@@ -1483,7 +1483,7 @@ private KafkaIndexTask createTask(
     final KafkaIndexTask task = new KafkaIndexTask(
         taskId,
         null,
-        DATA_SCHEMA,
+        cloneDataSchema(),
         tuningConfig,
         ioConfig,
         null,
@@ -1494,6 +1494,17 @@ private KafkaIndexTask createTask(
     return task;
   }
 
+  private static DataSchema cloneDataSchema()
+  {
+    return new DataSchema(
+        DATA_SCHEMA.getDataSource(),
+        DATA_SCHEMA.getParserMap(),
+        DATA_SCHEMA.getAggregators(),
+        DATA_SCHEMA.getGranularitySpec(),
+        objectMapper
+    );
+  }
+
   private QueryRunnerFactoryConglomerate makeTimeseriesOnlyConglomerate()
   {
     IntervalChunkingQueryRunnerDecorator queryRunnerDecorator = new IntervalChunkingQueryRunnerDecorator(

extensions-core/protobuf-extensions/src/main/java/io/druid/data/input/protobuf/ProtobufInputRowParser.java

@@ -47,10 +47,10 @@
 public class ProtobufInputRowParser implements ByteBufferInputRowParser
 {
   private final ParseSpec parseSpec;
-  private Parser<String, Object> parser;
   private final String descriptorFilePath;
   private final String protoMessageType;
-  private Descriptor descriptor;
+  private final Descriptor descriptor;
+  private Parser<String, Object> parser;
 
 
   @JsonCreator
@@ -63,7 +63,6 @@ public ProtobufInputRowParser(
     this.parseSpec = parseSpec;
     this.descriptorFilePath = descriptorFilePath;
     this.protoMessageType = protoMessageType;
-    this.parser = parseSpec.makeParser();
     this.descriptor = getDescriptor(descriptorFilePath);
   }
 
@@ -82,6 +81,11 @@ public ProtobufInputRowParser withParseSpec(ParseSpec parseSpec)
   @Override
   public InputRow parse(ByteBuffer input)
   {
+    if (parser == null) {
+      // parser should be created when it is really used to avoid unnecessary initialization of the underlying
+      // parseSpec.
+      parser = parseSpec.makeParser();
+    }
     String json;
     try {
       DynamicMessage message = DynamicMessage.parseFrom(descriptor, ByteString.copyFrom(input));