This is an automated email from the ASF dual-hosted git repository.
npeltier pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-pipes.git
The following commit(s) were added to refs/heads/master by this push:
new 52e54cf SLING-7632 add error report if any
52e54cf is described below
commit 52e54cf61be0db184bcc031564fd9eff95274428
Author: Nicolas Peltier <[email protected]>
AuthorDate: Thu May 24 17:56:25 2018 +0200
SLING-7632 add error report if any
- outputs nbErrors, and error array in case something happenened in json
output,
- outputs errors in csv format,
- added unit test & integration tests for json format
---
src/main/java/org/apache/sling/pipes/BasePipe.java | 37 ++++++++++++++++++----
.../org/apache/sling/pipes/ExecutionResult.java | 7 ++++
.../java/org/apache/sling/pipes/OutputWriter.java | 20 ++++++++++++
.../java/org/apache/sling/pipes/PipeBindings.java | 18 +++++++++++
.../org/apache/sling/pipes/internal/CsvWriter.java | 9 ++++++
.../apache/sling/pipes/internal/JsonWriter.java | 11 +++++++
.../apache/sling/pipes/internal/PlumberImpl.java | 14 ++++++++
.../java/org/apache/sling/pipes/BasePipeTest.java | 14 ++++++++
.../apache/sling/pipes/it/PlumberServletIT.java | 17 ++++++++--
.../SLING-INF/jcr_root/etc/pipes-it/bad-list.json | 19 +++++++++++
10 files changed, 158 insertions(+), 8 deletions(-)
diff --git a/src/main/java/org/apache/sling/pipes/BasePipe.java
b/src/main/java/org/apache/sling/pipes/BasePipe.java
index 511a1e1..24637cc 100644
--- a/src/main/java/org/apache/sling/pipes/BasePipe.java
+++ b/src/main/java/org/apache/sling/pipes/BasePipe.java
@@ -140,11 +140,18 @@ public class BasePipe implements Pipe {
}
/**
+ * @return configured input path (not computed)
+ */
+ protected String getRawPath() {
+ return properties.get(PN_PATH, "");
+ }
+
+ /**
* Get pipe's path, instanciated or not
* @return configured path (can be empty)
*/
public String getPath() throws ScriptException {
- String rawPath = properties.get(PN_PATH, "");
+ String rawPath = getRawPath();
return bindings.instantiateExpression(rawPath);
}
@@ -169,15 +176,25 @@ public class BasePipe implements Pipe {
return referrer == null ? (parent != null ?
parent.getPreviousPipe(this) : null) : referrer.getPreviousPipe();
}
- @Override
- public Resource getInput() throws ScriptException {
- Resource resource = getConfiguredInput();
- if (resource == null && parent != null){
+ /**
+ * @return previous pipe's output if in a container, null otherwise
+ */
+ protected Resource getPreviousResource(){
+ if (parent != null){
Pipe previousPipe = getPreviousPipe();
if (previousPipe != null) {
return bindings.getExecutedResource(previousPipe.getName());
}
}
+ return null;
+ }
+
+ @Override
+ public Resource getInput() throws ScriptException {
+ Resource resource = getConfiguredInput();
+ if (resource == null) {
+ resource = getPreviousResource();
+ }
logger.debug("input for this pipe is {}", resource != null ?
resource.getPath() : null);
return resource;
}
@@ -211,7 +228,15 @@ public class BasePipe implements Pipe {
try {
return computeOutput();
} catch (Exception e){
- logger.error("error with pipe execution ", e);
+ String path = getRawPath();
+ if (StringUtils.isBlank(path)){
+ Resource input = getPreviousResource();
+ if (input != null){
+ path = resource.getPath();
+ }
+ }
+ bindings.setCurrentError(path);
+ logger.error("error with pipe execution from {}", path, e);
}
return EMPTY_ITERATOR;
}
diff --git a/src/main/java/org/apache/sling/pipes/ExecutionResult.java
b/src/main/java/org/apache/sling/pipes/ExecutionResult.java
index 2f8a4a3..5831131 100644
--- a/src/main/java/org/apache/sling/pipes/ExecutionResult.java
+++ b/src/main/java/org/apache/sling/pipes/ExecutionResult.java
@@ -115,4 +115,11 @@ public class ExecutionResult {
}
return data;
}
+
+ /**
+ * @param error path to record
+ */
+ public void addError(String error) {
+ writer.error(error);
+ }
}
diff --git a/src/main/java/org/apache/sling/pipes/OutputWriter.java
b/src/main/java/org/apache/sling/pipes/OutputWriter.java
index ac32531..ef28c3c 100644
--- a/src/main/java/org/apache/sling/pipes/OutputWriter.java
+++ b/src/main/java/org/apache/sling/pipes/OutputWriter.java
@@ -26,7 +26,9 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.io.Writer;
+import java.util.ArrayList;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
/**
@@ -39,12 +41,18 @@ public abstract class OutputWriter {
public static final String KEY_ITEMS = "items";
+ public static final String KEY_ERRORS = "errors";
+
+ public static final String KEY_NB_ERRORS = "nbErrors";
+
public static final String PARAM_SIZE = KEY_SIZE;
public static final int NB_MAX = 10;
protected long size;
+ protected long nbErrors;
+
protected long max = NB_MAX;
protected Pipe pipe;
@@ -57,6 +65,7 @@ public abstract class OutputWriter {
protected Map<String, Object> customOutputs;
+ protected List<String> errors = new ArrayList<>();
/**
*
@@ -129,11 +138,22 @@ public abstract class OutputWriter {
}
/**
+ * Write a given error
+ * @param path resource that lead to the error
+ */
+ public void error(String path) {
+ if (nbErrors++ < max) {
+ errors.add(path);
+ }
+ }
+
+ /**
* Write a given resource
* @param resource resource that will be written
*/
protected abstract void writeItem(Resource resource);
+
/**
* writes the end of the output
*/
diff --git a/src/main/java/org/apache/sling/pipes/PipeBindings.java
b/src/main/java/org/apache/sling/pipes/PipeBindings.java
index 190732d..7f852af 100644
--- a/src/main/java/org/apache/sling/pipes/PipeBindings.java
+++ b/src/main/java/org/apache/sling/pipes/PipeBindings.java
@@ -82,6 +82,8 @@ public class PipeBindings {
Map<String, Resource> outputResources = new HashMap<>();
+ String currentError;
+
/**
* public constructor, built from pipe's resource
* @param resource pipe's configuration resource
@@ -326,4 +328,20 @@ public class PipeBindings {
nameBindings.put(name, resource.getName());
}
}
+
+ /**
+ * @return current error if any, and reset it
+ */
+ public String popCurrentError() {
+ String returnValue = currentError;
+ currentError = null;
+ return returnValue;
+ }
+
+ /**
+ * @param currentError error path to set
+ */
+ public void setCurrentError(String currentError) {
+ this.currentError = currentError;
+ }
}
diff --git a/src/main/java/org/apache/sling/pipes/internal/CsvWriter.java
b/src/main/java/org/apache/sling/pipes/internal/CsvWriter.java
index 49a38c5..46a3b76 100644
--- a/src/main/java/org/apache/sling/pipes/internal/CsvWriter.java
+++ b/src/main/java/org/apache/sling/pipes/internal/CsvWriter.java
@@ -16,6 +16,7 @@
*/
package org.apache.sling.pipes.internal;
+import com.sun.org.apache.bcel.internal.generic.NEW;
import org.apache.commons.lang3.StringUtils;
import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.SlingHttpServletResponse;
@@ -40,6 +41,8 @@ public class CsvWriter extends OutputWriter {
private static final String NEW_LINE = "\n";
+ private static final String HEADER_ERROR = "errors";
+
List<String> headers;
@Override
@@ -98,6 +101,12 @@ public class CsvWriter extends OutputWriter {
@Override
public void ends() {
try {
+ if (errors.size() > 0){
+ writer.write(HEADER_ERROR + NEW_LINE);
+ for (String error : errors){
+ writer.write(error + NEW_LINE);
+ }
+ }
writer.flush();
} catch (IOException e) {
LOG.error("unable to flush", e);
diff --git a/src/main/java/org/apache/sling/pipes/internal/JsonWriter.java
b/src/main/java/org/apache/sling/pipes/internal/JsonWriter.java
index 81ba1a3..822956a 100644
--- a/src/main/java/org/apache/sling/pipes/internal/JsonWriter.java
+++ b/src/main/java/org/apache/sling/pipes/internal/JsonWriter.java
@@ -25,10 +25,13 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import javax.json.Json;
+import javax.json.JsonArray;
import javax.json.JsonValue;
import javax.json.stream.JsonGenerator;
import javax.script.ScriptException;
import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Map;
/**
@@ -92,6 +95,14 @@ public class JsonWriter extends OutputWriter {
public void ends() {
jsonWriter.writeEnd();
jsonWriter.write(KEY_SIZE,size);
+ if (nbErrors > 0) {
+ jsonWriter.write(KEY_NB_ERRORS, nbErrors);
+ jsonWriter.writeStartArray(KEY_ERRORS);
+ for (String error : errors){
+ jsonWriter.write(error);
+ }
+ jsonWriter.writeEnd();
+ }
jsonWriter.writeEnd();
jsonWriter.flush();
}
diff --git a/src/main/java/org/apache/sling/pipes/internal/PlumberImpl.java
b/src/main/java/org/apache/sling/pipes/internal/PlumberImpl.java
index 9f8a564..ea64db9 100644
--- a/src/main/java/org/apache/sling/pipes/internal/PlumberImpl.java
+++ b/src/main/java/org/apache/sling/pipes/internal/PlumberImpl.java
@@ -272,12 +272,14 @@ public class PlumberImpl implements Plumber, JobConsumer,
PlumberMXBean {
ExecutionResult result = new ExecutionResult(writer);
for (Iterator<Resource> it = pipe.getOutput(); it.hasNext();){
Resource resource = it.next();
+ checkError(pipe, result);
if (resource != null) {
log.debug("[{}] retrieved {}", pipe.getName(),
resource.getPath());
result.addResultItem(resource);
persist(resolver, pipe, result, resource);
}
}
+ checkError(pipe, result);
if (save && pipe.modifiesContent()) {
persist(resolver, pipe, result, null);
}
@@ -299,6 +301,18 @@ public class PlumberImpl implements Plumber, JobConsumer,
PlumberMXBean {
}
/**
+ * check if current state contains error, and record it
+ * @param pipe current pipe
+ * @param result current result
+ */
+ protected void checkError(Pipe pipe, ExecutionResult result){
+ String error = pipe.getBindings().popCurrentError();
+ if (StringUtils.isNotBlank(error)){
+ result.addError(error);
+ }
+ }
+
+ /**
* Persists pipe change if big enough, or ended, and eventually distribute
changes
* @param resolver resolver to use
* @param pipe pipe at the origin of the changes,
diff --git a/src/test/java/org/apache/sling/pipes/BasePipeTest.java
b/src/test/java/org/apache/sling/pipes/BasePipeTest.java
index d5a3194..847e481 100644
--- a/src/test/java/org/apache/sling/pipes/BasePipeTest.java
+++ b/src/test/java/org/apache/sling/pipes/BasePipeTest.java
@@ -19,6 +19,12 @@ package org.apache.sling.pipes;
import org.apache.sling.api.resource.PersistenceException;
import org.junit.Test;
+import javax.json.Json;
+import javax.json.JsonArray;
+import javax.json.JsonObject;
+import java.io.StringReader;
+
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -39,4 +45,12 @@ public class BasePipeTest extends AbstractPipeTest {
assertTrue("Is dry run should be true with flag set to text true",
resetDryRun("true").isDryRun());
assertTrue("Is dry run should be true with flag set to something that
is not false or 'false'", resetDryRun("other").isDryRun());
}
+
+ @Test
+ public void simpleErrorTest() throws Exception {
+ ExecutionResult result =
plumber.newPipe(context.resourceResolver()).echo("${whatever is wrong}").run();
+ JsonObject response = Json.createReader(new
StringReader(result.toString())).readObject();
+ JsonArray array = response.getJsonArray(OutputWriter.KEY_ERRORS);
+ assertEquals("there should be one error", 1, array.size());
+ }
}
\ No newline at end of file
diff --git a/src/test/java/org/apache/sling/pipes/it/PlumberServletIT.java
b/src/test/java/org/apache/sling/pipes/it/PlumberServletIT.java
index 28d696a..b223425 100644
--- a/src/test/java/org/apache/sling/pipes/it/PlumberServletIT.java
+++ b/src/test/java/org/apache/sling/pipes/it/PlumberServletIT.java
@@ -36,8 +36,6 @@ import static org.junit.Assert.assertEquals;
@RunWith(PaxExam.class)
@ExamReactorStrategy(PerClass.class)
public class PlumberServletIT extends PipesTestSupport {
-
-
private static final Logger LOGGER =
LoggerFactory.getLogger(PlumberServletIT.class);
/**
@@ -45,7 +43,9 @@ public class PlumberServletIT extends PipesTestSupport {
*/
class ExpectedResponse {
int size;
+ int nbErrors;
ArrayList<String> items;
+ ArrayList<String> errors;
}
@Test
@@ -59,5 +59,18 @@ public class PlumberServletIT extends PipesTestSupport {
assertArrayEquals("should be fruits array", new String[]
{"/content/fruits/apple", "/content/fruits/banana"}, json.items.toArray());
}
+ @Test
+ public void testErrors() throws IOException {
+ final String url =
String.format("http://localhost:%s/etc/pipes-it/bad-list.json";, httpPort());
+ LOGGER.info("fetching {}", url);
+ final String response = Jsoup.connect(url).header("Authorization",
basicAuthorizationHeader(ADMIN_CREDENTIALS)).ignoreContentType(true).execute().body();
+ LOGGER.info("retrieved following response {}", response);
+ final ExpectedResponse json = (new Gson()).fromJson(response, new
TypeToken<ExpectedResponse>(){}.getType());
+ assertEquals("there should be 1 elements", 1, json.size);
+ assertArrayEquals("should be single apple array", new String[]
{"/content/fruits/apple"}, json.items.toArray());
+ assertEquals("there should be 1 error", 1, json.nbErrors);
+ assertArrayEquals("should be single error array", new String[]
{"${name.list === 'apple' ? path.apple : unexistingVariable}"},
json.errors.toArray());
+ }
+
}
diff --git a/src/test/resources/SLING-INF/jcr_root/etc/pipes-it/bad-list.json
b/src/test/resources/SLING-INF/jcr_root/etc/pipes-it/bad-list.json
new file mode 100644
index 0000000..5edae33
--- /dev/null
+++ b/src/test/resources/SLING-INF/jcr_root/etc/pipes-it/bad-list.json
@@ -0,0 +1,19 @@
+{
+ "jcr:primaryType":"nt:unstructured",
+ "jcr:description":"returns fruit list under /content/fruits",
+ "sling:resourceType":"slingPipes/container",
+ "conf":{
+ "jcr:primaryType":"sling:OrderedFolder",
+ "list": {
+ "jcr:primaryType":"nt:unstructured",
+ "jcr:description":"returns fruit list under /content/fruits",
+ "sling:resourceType":"slingPipes/children",
+ "path":"/content/fruits",
+ "expr":"nt:base"
+ },
+ "echo": {
+ "sling:resourceType":"slingPipes/base",
+ "path":"${name.list === 'apple' ? path.apple : unexistingVariable}"
+ }
+ }
+}
\ No newline at end of file
--
To stop receiving notification emails like this one, please contact
[email protected].
