Remove need for AbstractDocumentTransform
Names are now managed by the TransformPipeline instead of individual
DocumentTransforms.
This change primarily serves simplifying the transform interface as well
as transform implementations. However, it also prevents a buggy
transform from mis-representing its name.
CommandLineTransform has been converted away from
AbstractDocumentTransform, but its initialization process could still be
simplified considerably.
diff --git a/src/com/google/enterprise/adaptor/AbstractDocumentTransform.java b/src/com/google/enterprise/adaptor/AbstractDocumentTransform.java
deleted file mode 100644
index 882614d..0000000
--- a/src/com/google/enterprise/adaptor/AbstractDocumentTransform.java
+++ /dev/null
@@ -1,62 +0,0 @@
-// Copyright 2011 Google Inc. All Rights Reserved.
-//
-// Licensed 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 com.google.enterprise.adaptor;
-
-import java.util.Map;
-
-/**
- * Convenience class for implementing {@code DocumentTransform}s.
- * Implementations only need to implement {@link #transform}, although they
- * should also likely have a static factory method as defined in {@link
- * DocumentTransform}.
- */
-public abstract class AbstractDocumentTransform implements DocumentTransform {
- private String name = getClass().getName();
-
- public AbstractDocumentTransform() {}
-
- /**
- * If {@code name} is {@code null}, the default is used.
- */
- public AbstractDocumentTransform(String name) {
- if (name != null) {
- this.name = name;
- }
- }
-
- /**
- * Configure this instance with provided {@code config}. Accepts key {@code
- * "name"}. Unknown keys are ignored. This method is
- * intended as a convenience for use in a static factory method.
- */
- protected void configure(Map<String, String> config) {
- String name = config.get("name");
- if (name != null) {
- this.name = name;
- }
- }
-
- protected void setName(String name) {
- if (name == null) {
- throw new NullPointerException();
- }
- this.name = name;
- }
-
- @Override
- public String getName() {
- return name;
- }
-}
diff --git a/src/com/google/enterprise/adaptor/DocumentTransform.java b/src/com/google/enterprise/adaptor/DocumentTransform.java
index 2f6526f..02b1559 100644
--- a/src/com/google/enterprise/adaptor/DocumentTransform.java
+++ b/src/com/google/enterprise/adaptor/DocumentTransform.java
@@ -14,9 +14,6 @@
package com.google.enterprise.adaptor;
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-import java.io.OutputStream;
import java.util.Map;
/**
@@ -24,8 +21,7 @@
*
* <p>Implementations should also typically have a static factory method with a
* single {@code Map<String, String>} argument for creating instances based on
- * configuration. Implementations are encouraged to accept "name" as a
- * configuration key.
+ * configuration.
*/
public interface DocumentTransform {
/**
@@ -37,11 +33,4 @@
*/
public void transform(Metadata metadata, Map<String, String> params)
throws TransformException;
-
- /**
- * The name of this transform instance, typically provided by the user. It
- * should not be {@code null}. Using the class name as a default is reasonable
- * if no name has been provided.
- */
- public String getName();
}
diff --git a/src/com/google/enterprise/adaptor/GsaCommunicationHandler.java b/src/com/google/enterprise/adaptor/GsaCommunicationHandler.java
index f50ab3b..9fb3a7f 100644
--- a/src/com/google/enterprise/adaptor/GsaCommunicationHandler.java
+++ b/src/com/google/enterprise/adaptor/GsaCommunicationHandler.java
@@ -347,6 +347,7 @@
static TransformPipeline createTransformPipeline(
List<Map<String, String>> pipelineConfig) {
List<DocumentTransform> elements = new LinkedList<DocumentTransform>();
+ List<String> names = new LinkedList<String>();
for (Map<String, String> element : pipelineConfig) {
final String name = element.get("name");
final String confPrefix = "transform.pipeline." + name + ".";
@@ -392,9 +393,10 @@
}
DocumentTransform transform = (DocumentTransform) o;
elements.add(transform);
+ names.add(name);
}
// If we created an empty pipeline, then we don't need the pipeline at all.
- return elements.size() > 0 ? new TransformPipeline(elements) : null;
+ return elements.size() > 0 ? new TransformPipeline(elements, names) : null;
}
/**
diff --git a/src/com/google/enterprise/adaptor/TransformPipeline.java b/src/com/google/enterprise/adaptor/TransformPipeline.java
index 988f83f..b411d15 100644
--- a/src/com/google/enterprise/adaptor/TransformPipeline.java
+++ b/src/com/google/enterprise/adaptor/TransformPipeline.java
@@ -14,17 +14,11 @@
package com.google.enterprise.adaptor;
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-import java.io.OutputStream;
-import java.io.UnsupportedEncodingException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.logging.Level;
-import java.util.logging.Logger;
/**
* Modify metadata using multiple serial transforms. The transforms
@@ -34,13 +28,21 @@
* <p>This class is thread-safe.
*/
public class TransformPipeline {
- private static final Logger log
- = Logger.getLogger(TransformPipeline.class.getName());
-
private final List<DocumentTransform> transformList;
+ private final List<String> names;
- public TransformPipeline(List<? extends DocumentTransform> transforms) {
+ public TransformPipeline(List<? extends DocumentTransform> transforms, List<String> names) {
+ if (transforms.size() != names.size()) {
+ throw new IllegalArgumentException("Transforms and names must be the same size");
+ }
+ if (transforms.contains(null)) {
+ throw new NullPointerException("Transforms must not contain null values");
+ }
+ if (names.contains(null)) {
+ throw new NullPointerException("Names must not contain null values");
+ }
this.transformList = Collections.unmodifiableList(new ArrayList<DocumentTransform>(transforms));
+ this.names = Collections.unmodifiableList(new ArrayList<String>(names));
}
/**
@@ -56,11 +58,12 @@
Map<String, String> paramsInTransit = Collections.checkedMap(
new HashMap<String, String>(params), String.class, String.class);
- for (DocumentTransform transform : transformList) {
+ for (int i = 0; i < transformList.size(); i++) {
+ DocumentTransform transform = transformList.get(i);
try {
transform.transform(metadataInTransit, paramsInTransit);
} catch (TransformException e) {
- throw new TransformException("Aborting " + transform.getName(), e);
+ throw new TransformException("Aborting " + names.get(i), e);
}
}
@@ -75,4 +78,8 @@
public List<DocumentTransform> getDocumentTransforms() {
return transformList;
}
+
+ public List<String> getNames() {
+ return names;
+ }
}
diff --git a/src/com/google/enterprise/adaptor/prebuilt/CommandLineTransform.java b/src/com/google/enterprise/adaptor/prebuilt/CommandLineTransform.java
index 4b282e5..85d81f8 100644
--- a/src/com/google/enterprise/adaptor/prebuilt/CommandLineTransform.java
+++ b/src/com/google/enterprise/adaptor/prebuilt/CommandLineTransform.java
@@ -16,7 +16,7 @@
import static java.util.AbstractMap.SimpleEntry;
-import com.google.enterprise.adaptor.AbstractDocumentTransform;
+import com.google.enterprise.adaptor.DocumentTransform;
import com.google.enterprise.adaptor.IOHelper;
import com.google.enterprise.adaptor.Metadata;
import com.google.enterprise.adaptor.TransformException;
@@ -30,7 +30,7 @@
* A conduit that allows a simple way to create a document transform based on
* a command line program.
*/
-public class CommandLineTransform extends AbstractDocumentTransform {
+public class CommandLineTransform implements DocumentTransform {
private static final Logger log
= Logger.getLogger(CommandLineTransform.class.getName());
private static final int STDERR_BUFFER_SIZE = 51200; // 50 kB
@@ -43,13 +43,12 @@
public CommandLineTransform() {}
/**
- * Accepts keys {@code "cmd"}, {@code "workingDirectory"}, {@code "arg?"}, and
- * any keys accepted by the super class. The {@code "arg?"} configuration
- * values should be numerically increasing starting from one: {@code "arg1"},
- * {@code "arg2"}, {@code "arg3}, ...
+ * Accepts keys {@code "cmd"}, {@code "workingDirectory"}, and {@code "arg?"}.
+ * The {@code "arg?"} configuration values should be numerically increasing
+ * starting from one: {@code "arg1"}, {@code "arg2"}, {@code "arg3}, ...
*/
- protected void configure(Map<String, String> config) {
- super.configure(config);
+ public static CommandLineTransform create(Map<String, String> config) {
+ CommandLineTransform transform = new CommandLineTransform();
List<String> cmdList = new ArrayList<String>();
String cmd = config.get("cmd");
@@ -61,12 +60,12 @@
String workingDirectory = config.get("workingDirectory");
if (workingDirectory != null) {
- setWorkingDirectory(new File(workingDirectory));
+ transform.setWorkingDirectory(new File(workingDirectory));
}
String cmdAcceptsParameters = config.get("cmdAcceptsParameters");
if (cmdAcceptsParameters != null) {
- this.commandAcceptsParameters
+ transform.commandAcceptsParameters
= Boolean.parseBoolean(cmdAcceptsParameters);
}
@@ -77,7 +76,8 @@
}
cmdList.add(value);
}
- transformCommand = cmdList;
+ transform.transformCommand = cmdList;
+ return transform;
}
@Override
@@ -241,15 +241,4 @@
public File getWorkingDirectory() {
return workingDirectory;
}
-
- @Override
- public void setName(String name) {
- super.setName(name);
- }
-
- public static CommandLineTransform create(Map<String, String> config) {
- CommandLineTransform transform = new CommandLineTransform();
- transform.configure(config);
- return transform;
- }
}
diff --git a/test/com/google/enterprise/adaptor/DocumentHandlerTest.java b/test/com/google/enterprise/adaptor/DocumentHandlerTest.java
index 89c2f01..e122602 100644
--- a/test/com/google/enterprise/adaptor/DocumentHandlerTest.java
+++ b/test/com/google/enterprise/adaptor/DocumentHandlerTest.java
@@ -323,14 +323,15 @@
public void testTransform() throws Exception {
final String key = "testing key";
List<DocumentTransform> transforms = new LinkedList<DocumentTransform>();
- transforms.add(new AbstractDocumentTransform() {
+ transforms.add(new DocumentTransform() {
@Override
public void transform(Metadata metadata, Map<String, String> params) {
metadata.set(key, metadata.getOneValue(key).toUpperCase());
metadata.set("docid", params.get("DocId"));
}
});
- TransformPipeline transform = new TransformPipeline(transforms);
+ TransformPipeline transform = new TransformPipeline(transforms,
+ Arrays.asList("t1"));
mockAdaptor = new MockAdaptor() {
@Override
public void getDocContent(Request request, Response response)
diff --git a/test/com/google/enterprise/adaptor/GsaCommunicationHandlerTest.java b/test/com/google/enterprise/adaptor/GsaCommunicationHandlerTest.java
index f70d632..716d461 100644
--- a/test/com/google/enterprise/adaptor/GsaCommunicationHandlerTest.java
+++ b/test/com/google/enterprise/adaptor/GsaCommunicationHandlerTest.java
@@ -270,7 +270,7 @@
assertEquals(1, pipeline.getDocumentTransforms().size());
assertEquals(IdentityTransform.class,
pipeline.getDocumentTransforms().get(0).getClass());
- assertEquals("testing", pipeline.getDocumentTransforms().get(0).getName());
+ assertEquals("testing", pipeline.getNames().get(0));
}
@Test
@@ -415,16 +415,14 @@
}
}
- static class IdentityTransform extends AbstractDocumentTransform {
+ static class IdentityTransform implements DocumentTransform {
@Override
public void transform(Metadata metadata, Map<String, String> params) {
}
}
public static IdentityTransform factoryMethod(Map<String, String> config) {
- IdentityTransform transform = new IdentityTransform();
- transform.configure(config);
- return transform;
+ return new IdentityTransform();
}
public static IdentityTransform wrongFactoryMethod() {
diff --git a/test/com/google/enterprise/adaptor/TransformPipelineTest.java b/test/com/google/enterprise/adaptor/TransformPipelineTest.java
index 3ed5c91..611a70b 100644
--- a/test/com/google/enterprise/adaptor/TransformPipelineTest.java
+++ b/test/com/google/enterprise/adaptor/TransformPipelineTest.java
@@ -19,9 +19,7 @@
import org.junit.*;
import org.junit.rules.ExpectedException;
-import java.io.ByteArrayOutputStream;
import java.io.IOException;
-import java.io.OutputStream;
import java.util.*;
/**
@@ -33,7 +31,8 @@
@Test
public void testNoOpEmpty() throws IOException, TransformException {
- TransformPipeline pipeline = new TransformPipeline(Collections.<DocumentTransform>emptyList());
+ TransformPipeline pipeline = new TransformPipeline(
+ Collections.<DocumentTransform>emptyList(), Collections.<String>emptyList());
Metadata metadata = new Metadata();
Map<String, String> params = new HashMap<String, String>();
pipeline.transform(metadata, params);
@@ -44,7 +43,8 @@
@Test
public void testNoOpWithInput() throws IOException, TransformException {
- TransformPipeline pipeline = new TransformPipeline(Collections.<DocumentTransform>emptyList());
+ TransformPipeline pipeline = new TransformPipeline(
+ Collections.<DocumentTransform>emptyList(), Collections.<String>emptyList());
Metadata metadata = new Metadata();
metadata.add("key1", "value1");
Map<String, String> params = new HashMap<String, String>();
@@ -65,14 +65,15 @@
params.put("key2", "value2");
List<DocumentTransform> transforms = new ArrayList<DocumentTransform>();
- transforms.add(new AbstractDocumentTransform() {
+ transforms.add(new DocumentTransform() {
@Override
public void transform(Metadata m, Map<String, String> p) throws TransformException {
m.set("newMeta", "metaValue");
p.put("newKey", "newValue");
}
});
- TransformPipeline pipeline = new TransformPipeline(transforms);
+ TransformPipeline pipeline = new TransformPipeline(
+ transforms, Arrays.asList("t1"));
pipeline.transform(metadata, params);
assertEquals("value1", metadata.getOneValue("key1"));
@@ -83,10 +84,7 @@
assertEquals(2, params.size());
}
- private static class ErroringTransform extends AbstractDocumentTransform {
- public ErroringTransform() {
- super(null);
- }
+ private static class ErroringTransform implements DocumentTransform {
@Override
public void transform(Metadata metadata, Map<String, String> p)
throws TransformException {
@@ -97,7 +95,7 @@
}
}
- private static class IncrementTransform extends AbstractDocumentTransform {
+ private static class IncrementTransform implements DocumentTransform {
@Override
public void transform(Metadata metadata, Map<String, String> p)
throws TransformException {
@@ -106,7 +104,7 @@
}
}
- private static class ProductTransform extends AbstractDocumentTransform {
+ private static class ProductTransform implements DocumentTransform {
private int factor;
public ProductTransform(int factor) {
@@ -123,7 +121,8 @@
@Test
public void testTransform() throws IOException, TransformException {
- TransformPipeline pipeline = new TransformPipeline(Arrays.asList(new IncrementTransform()));
+ TransformPipeline pipeline = new TransformPipeline(
+ Arrays.asList(new IncrementTransform()), Arrays.asList("it"));
Metadata metadata = new Metadata();
metadata.add("int", "0");
Map<String, String> params = new HashMap<String, String>();
@@ -140,7 +139,8 @@
@Test
public void testMultipleTransforms() throws IOException, TransformException {
TransformPipeline pipeline = new TransformPipeline(Arrays.asList(
- new IncrementTransform(), new ProductTransform(2)));
+ new IncrementTransform(), new ProductTransform(2)),
+ Arrays.asList("it", "pt"));
Metadata metadata = new Metadata();
metadata.set("int", "0");
@@ -158,7 +158,8 @@
@Test
public void testTransformErrorFatal() throws IOException, TransformException {
TransformPipeline pipeline = new TransformPipeline(Arrays.asList(
- new IncrementTransform(), new ErroringTransform()));
+ new IncrementTransform(), new ErroringTransform()),
+ Arrays.asList("it", "et"));
Metadata metadata = new Metadata();
metadata.set("int", "0");
Map<String, String> params = new HashMap<String, String>();
diff --git a/test/com/google/enterprise/adaptor/prebuilt/CommandLineTransformTest.java b/test/com/google/enterprise/adaptor/prebuilt/CommandLineTransformTest.java
index effeab8..e9e95fd 100644
--- a/test/com/google/enterprise/adaptor/prebuilt/CommandLineTransformTest.java
+++ b/test/com/google/enterprise/adaptor/prebuilt/CommandLineTransformTest.java
@@ -19,7 +19,6 @@
import com.google.enterprise.adaptor.Metadata;
import com.google.enterprise.adaptor.TestHelper;
import com.google.enterprise.adaptor.TransformException;
-import com.google.enterprise.adaptor.TransformPipeline;
import org.junit.Test;
@@ -44,8 +43,7 @@
CommandLineTransform cmd = new CommandLineTransform();
cmd.setTransformCommand(Arrays.asList(new String[] {"sed", "s/i/1/"}));
cmd.setCommandAcceptsParameters(false);
- TransformPipeline pipeline = new TransformPipeline(Arrays.asList(cmd));
- pipeline.transform(metadata, params);
+ cmd.transform(metadata, params);
assertEquals("metaValue1", metadata.getOneValue("metaKey1"));
assertEquals(1, metadata.getKeys().size());
@@ -74,8 +72,7 @@
+ "rm \"$TMPFILE\" >&2;"
}));
cmd.setCommandAcceptsParameters(true);
- TransformPipeline pipeline = new TransformPipeline(Arrays.asList(cmd));
- pipeline.transform(metadata, params);
+ cmd.transform(metadata, params);
assertEquals(1, metadata.getKeys().size());
assertEquals("metaValue2", metadata.getOneValue("metaKey2"));
