#95 Rethrowing REST module failures

Author: rjrudin

gradle.properties

@@ -1,3 +1,3 @@
 group=com.marklogic
 javadocsDir=../gh-pages-marklogic-java/javadocs
-version=3.6.0
+version=3.7.0-dev

src/main/java/com/marklogic/client/ext/modulesloader/impl/DefaultModulesLoader.java

@@ -28,11 +28,27 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.*;
+import java.util.function.Supplier;
 import java.util.regex.Pattern;
 
 /**
- * Default implementation of ModulesLoader. Loads everything except assets via the REST API. Assets are either loaded
- * via an XccAssetLoader (faster) or via a RestApiAssetLoader (slower, but doesn't require additional privileges).
+ * Default implementation of ModulesLoader.
+ * <p>
+ * REST modules and non-REST modules are handled in different ways. Non-REST modules are loaded via an instance of
+ * AssetFileLoader, which most likely loads modules via port 8000, directly into the desired modules database.
+ * </p>
+ * <p>
+ * REST modules however have to be loaded via the REST server that they're targeted for. This class also loads them
+ * by default via multiple threads to increase performance, as it can take a couple seconds to load each set of
+ * search options, service, and transform.
+ * </p>
+ * <p>
+ * To handle errors while loading REST modules in parallel, an implementation of LoadModulesFailureListener can be
+ * added to this class (ideally would have just been a Consumer that receives a Throwable). By default, an instance of
+ * SimpleLoadModulesFailureListener is added. Also by default, any Throwable caught by this class will be rethrown
+ * after all REST modules have been loaded. This behavior can be disabled by setting rethrowRestModulesFailure to
+ * false, in which case the failures are just logged.
+ * </p>
  */
 public class DefaultModulesLoader extends LoggingObject implements ModulesLoader {
 
@@ -52,6 +68,7 @@ public class DefaultModulesLoader extends LoggingObject implements ModulesLoader
 	private TokenReplacer tokenReplacer;
 
 	private List<LoadModulesFailureListener> failureListeners = new ArrayList<>();
+	private boolean rethrowRestModulesFailure = true;
 
 	/**
 	 * When set to true, exceptions thrown while loading transforms and resources will be caught and logged, and the
@@ -136,6 +153,7 @@ public Set<Resource> loadModules(String baseDir, ModulesFinder modulesFinder, Da
 		loadResources(modules, loadedModules);
 
 		waitForTaskExecutorToFinish();
+		rethrowRestModulesFailureIfOneExists();
 
 		if (logger.isDebugEnabled()) {
 			logger.debug("Finished loading modules from base directory: " + baseDir);
@@ -144,6 +162,20 @@ public Set<Resource> loadModules(String baseDir, ModulesFinder modulesFinder, Da
 		return loadedModules;
 	}
 
+	protected void rethrowRestModulesFailureIfOneExists() {
+		if (failureListeners != null && rethrowRestModulesFailure) {
+			for (LoadModulesFailureListener listener : failureListeners) {
+				if (listener instanceof Supplier) {
+					Object o = ((Supplier) listener).get();
+					if (o instanceof Throwable) {
+						Throwable t = (Throwable) o;
+						throw new RuntimeException("Error occurred while loading REST modules: " + t.getMessage(), t);
+					}
+				}
+			}
+		}
+	}
+
 	/**
 	 * If an AsyncTaskExecutor is used for loading options/services/transforms, we need to wait for the tasks to complete
 	 * before we e.g. release the DatabaseClient.
@@ -273,12 +305,12 @@ protected void applyXmlProperties(ServerConfigurationManager mgr, Resource r, Fi
 	protected File getFileFromResource(Resource r) {
 		try {
 			return r.getFile();
-		} catch (IOException ex) {}
+		} catch (IOException ex) {
+		}
 		return null;
 	}
 
 	/**
-	 *
 	 * @param modules
 	 * @param loadedModules
 	 */
@@ -306,7 +338,7 @@ protected void loadAssets(Modules modules, Set<Resource> loadedModules) {
 			assetFileLoader.initializeDocumentFileReader();
 			DocumentFileReader dfr = assetFileLoader.getDocumentFileReader();
 			if (dfr instanceof DefaultDocumentFileReader) {
-				DefaultDocumentFileReader reader = (DefaultDocumentFileReader)dfr;
+				DefaultDocumentFileReader reader = (DefaultDocumentFileReader) dfr;
 				reader.addDocumentFileProcessor(documentFile -> {
 					File f = documentFile.getFile();
 					if (f == null) {
@@ -339,7 +371,6 @@ protected void loadAssets(Modules modules, Set<Resource> loadedModules) {
 	}
 
 	/**
-	 *
 	 * @param modules
 	 * @param loadedModules
 	 */
@@ -356,7 +387,6 @@ protected void loadQueryOptions(Modules modules, Set<Resource> loadedModules) {
 	}
 
 	/**
-	 *
 	 * @param modules
 	 * @param loadedModules
 	 */
@@ -385,7 +415,6 @@ protected void loadTransforms(Modules modules, Set<Resource> loadedModules) {
 	}
 
 	/**
-	 *
 	 * @param modules
 	 * @param loadedModules
 	 */
@@ -414,7 +443,6 @@ protected void loadResources(Modules modules, Set<Resource> loadedModules) {
 	}
 
 	/**
-	 *
 	 * @param modules
 	 * @param loadedModules
 	 */
@@ -431,7 +459,6 @@ protected void loadNamespaces(Modules modules, Set<Resource> loadedModules) {
 	}
 
 	/**
-	 *
 	 * @param r
 	 * @param metadata
 	 * @param methodParams
@@ -456,7 +483,6 @@ public Resource installService(Resource r, final ExtensionMetadata metadata, fin
 	}
 
 	/**
-	 *
 	 * @param r
 	 * @param metadata
 	 * @return
@@ -472,21 +498,20 @@ public Resource installTransform(Resource r, final ExtensionMetadata metadata) {
 
 		StringHandle h = new StringHandle(readAndReplaceTokens(r));
 		executeTask(() -> {
-            if (FilenameUtil.isXslFile(filename)) {
-                mgr.writeXSLTransform(transformName, h, metadata);
-            } else if (FilenameUtil.isJavascriptFile(filename)) {
-                mgr.writeJavascriptTransform(transformName, h, metadata);
-            } else {
-                mgr.writeXQueryTransform(transformName, h, metadata);
-            }
-        });
+			if (FilenameUtil.isXslFile(filename)) {
+				mgr.writeXSLTransform(transformName, h, metadata);
+			} else if (FilenameUtil.isJavascriptFile(filename)) {
+				mgr.writeJavascriptTransform(transformName, h, metadata);
+			} else {
+				mgr.writeXQueryTransform(transformName, h, metadata);
+			}
+		});
 		updateTimestamp(r);
 
 		return r;
 	}
 
 	/**
-	 *
 	 * @param r
 	 * @return
 	 */
@@ -502,12 +527,12 @@ public Resource installQueryOptions(Resource r) {
 
 		StringHandle h = new StringHandle(readAndReplaceTokens(r));
 		executeTask(() -> {
-            if (filename.endsWith(".json")) {
-                mgr.writeOptions(name, h.withFormat(Format.JSON));
-            } else {
-                mgr.writeOptions(name, h);
-            }
-        });
+			if (filename.endsWith(".json")) {
+				mgr.writeOptions(name, h.withFormat(Format.JSON));
+			} else {
+				mgr.writeOptions(name, h);
+			}
+		});
 		updateTimestamp(r);
 		return r;
 	}
@@ -543,15 +568,13 @@ protected void executeTask(Runnable r) {
 		taskExecutor.execute(() -> {
 			try {
 				r.run();
-			}
-			catch(Exception e) {
+			} catch (Exception e) {
 				failureListeners.forEach(listener -> listener.processFailure(e));
 			}
 		});
 	}
 
 	/**
-	 *
 	 * @param r
 	 * @return
 	 */
@@ -582,7 +605,6 @@ public Resource installNamespace(Resource r) {
 	}
 
 	/**
-	 *
 	 * @param r
 	 * @return
 	 */
@@ -685,7 +707,8 @@ private boolean hasFileBeenModified(Resource resource) {
 			try {
 				File file = resource.getFile();
 				modified = modulesManager.hasFileBeenModifiedSinceLastLoaded(file);
-			} catch (IOException e) {}
+			} catch (IOException e) {
+			}
 		}
 		return modified;
 	}
@@ -695,7 +718,8 @@ private void updateTimestamp(Resource resource) {
 			try {
 				File file = resource.getFile();
 				modulesManager.saveLastLoadedTimestamp(file, new Date());
-			} catch (IOException e) {}
+			} catch (IOException e) {
+			}
 		}
 	}
 
@@ -718,4 +742,8 @@ public void setTokenReplacer(TokenReplacer tokenReplacer) {
 	public void setIncludeFilenamePattern(Pattern includeFilenamePattern) {
 		this.includeFilenamePattern = includeFilenamePattern;
 	}
+
+	public void setRethrowRestModulesFailure(boolean rethrowRestModulesFailure) {
+		this.rethrowRestModulesFailure = rethrowRestModulesFailure;
+	}
 }

src/main/java/com/marklogic/client/ext/modulesloader/impl/LoadModulesFailureListener.java

@@ -1,5 +1,11 @@
 package com.marklogic.client.ext.modulesloader.impl;
 
+/**
+ * Ideally this would just have been a Consumer that receives an instance of Throwable. And also, it's only for
+ * loading REST modules, which DefaultModulesLoader loads by default in parallel.
+ */
 public interface LoadModulesFailureListener {
+
 	void processFailure(Throwable throwable);
+
 }

src/main/java/com/marklogic/client/ext/modulesloader/impl/SimpleLoadModulesFailureListener.java

@@ -2,12 +2,24 @@
 
 import com.marklogic.client.ext.helper.LoggingObject;
 
-public class SimpleLoadModulesFailureListener extends LoggingObject implements LoadModulesFailureListener {
+import java.util.function.Supplier;
+
+public class SimpleLoadModulesFailureListener extends LoggingObject implements LoadModulesFailureListener, Supplier<Throwable> {
+
+	private Throwable firstThrowable;
 
 	@Override
 	public void processFailure(Throwable throwable) {
+		if (firstThrowable == null) {
+			firstThrowable = throwable;
+		}
 		if (logger.isErrorEnabled()) {
 			logger.error("Error caught while loading modules, cause: " + throwable.getMessage(), throwable);
 		}
 	}
+
+	@Override
+	public Throwable get() {
+		return firstThrowable;
+	}
 }

src/test/java/com/marklogic/client/ext/modulesloader/impl/LoadModulesTest.java

@@ -117,6 +117,22 @@ public void pathWithSpaces() {
 		assertEquals("<example/>", moduleXml.trim());
 	}
 
+	@Test
+	public void invalidRestModule() {
+		String dir = Paths.get("src", "test", "resources", "invalid-rest-modules").toString();
+
+		try {
+			modulesLoader.loadModules(dir, new DefaultModulesFinder(), client);
+			fail("Loading modules should have failed because of an invalid REST options file");
+		} catch (RuntimeException re) {
+			assertTrue(re.getMessage().contains("Unexpected character"));
+		}
+
+		// This should now succeed since DefaultModulesLoader won't rethrow the REST module failure
+		modulesLoader.setRethrowRestModulesFailure(false);
+		modulesLoader.loadModules(dir, new DefaultModulesFinder(), client);
+	}
+
 	/**
 	 * This test is a little brittle because it assumes the URI of options/services/transforms that are loaded
 	 * into the Modules database.

src/test/resources/invalid-rest-modules/options/invalid-options.xml

@@ -0,0 +1,3 @@
+<options xmlns="http://marklogic.com/appservices/search">
+	<TheseAre>invalid</TheseAre
+</options>