From 5ab5f9093f2d85348996691e916c2bf725044872 Mon Sep 17 00:00:00 2001 From: Martin Davis Date: Thu, 15 Aug 2019 15:18:40 -0700 Subject: [PATCH] Improve JTSOp command - error handling, testing (#459) * Improve JTSOpCmd to make it testable * Improve error handling * Update Travis CI with dist: trusty as per JDK8 failing advice Signed-off-by: Martin Davis --- .travis.yml | 2 + .../jtstest/cmd/CommandError.java | 10 ++ .../jtstest/cmd/CommandOutput.java | 48 ++++++ .../locationtech/jtstest/cmd/JTSOpCmd.java | 154 ++++++++++++------ .../jtstest/cmd/JTSOpCmdTest.java | 138 ++++++++++++++++ 5 files changed, 304 insertions(+), 48 deletions(-) create mode 100644 modules/app/src/main/java/org/locationtech/jtstest/cmd/CommandError.java create mode 100644 modules/app/src/main/java/org/locationtech/jtstest/cmd/CommandOutput.java create mode 100644 modules/app/src/test/java/org/locationtech/jtstest/cmd/JTSOpCmdTest.java diff --git a/.travis.yml b/.travis.yml index 20e9a0bcd7..6d037e3607 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,3 +6,5 @@ cache: directories: - $HOME/.m2 install: mvn install -B -V +dist: trusty + diff --git a/modules/app/src/main/java/org/locationtech/jtstest/cmd/CommandError.java b/modules/app/src/main/java/org/locationtech/jtstest/cmd/CommandError.java new file mode 100644 index 0000000000..f5342d9dea --- /dev/null +++ b/modules/app/src/main/java/org/locationtech/jtstest/cmd/CommandError.java @@ -0,0 +1,10 @@ +package org.locationtech.jtstest.cmd; + +public class CommandError extends RuntimeException { + public CommandError(String msg) { + super(msg); + } + public CommandError(String msg, String val) { + super(msg + ": " + val); + } +} diff --git a/modules/app/src/main/java/org/locationtech/jtstest/cmd/CommandOutput.java b/modules/app/src/main/java/org/locationtech/jtstest/cmd/CommandOutput.java new file mode 100644 index 0000000000..d149f7edbe --- /dev/null +++ b/modules/app/src/main/java/org/locationtech/jtstest/cmd/CommandOutput.java @@ -0,0 +1,48 @@ +package org.locationtech.jtstest.cmd; + +public class CommandOutput { + + private StringBuilder output = new StringBuilder(); + private boolean isCapture = false; + + public CommandOutput() { + + } + + public CommandOutput(boolean isCapture) { + this.isCapture = true; + } + + public void println() { + if (isCapture ) { + output.append("\n"); + } + else { + System.out.println(); + } + } + + public void println(Object o) { + if (isCapture ) { + output.append(o); + output.append("\n"); + } + else { + System.out.println(o); + } + } + + public void print(String s) { + if (isCapture ) { + output.append(s); + } + else { + System.out.print(s); + } + } + + public String getOutput() { + return output.toString(); + } + +} diff --git a/modules/app/src/main/java/org/locationtech/jtstest/cmd/JTSOpCmd.java b/modules/app/src/main/java/org/locationtech/jtstest/cmd/JTSOpCmd.java index 794fa21125..64c34332b7 100644 --- a/modules/app/src/main/java/org/locationtech/jtstest/cmd/JTSOpCmd.java +++ b/modules/app/src/main/java/org/locationtech/jtstest/cmd/JTSOpCmd.java @@ -11,9 +11,9 @@ */ package org.locationtech.jtstest.cmd; +import java.io.FileNotFoundException; import java.io.IOException; import java.util.Collection; -import java.util.Iterator; import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.GeometryFactory; @@ -36,7 +36,7 @@ import org.locationtech.jtstest.util.io.MultiFormatReader; /** - * A simple CLI to run TestBuilder operations. + * A CLI to run JTS TestBuilder operations. * Allows easier execution of JTS functions on test data for debugging purposes. *

* Examples: @@ -69,6 +69,13 @@ public class JTSOpCmd { // TODO: add option -ab to read both geoms from a file // TODO: allow -a stdin to indicate reading from stdin. + public static final String ERR_FILE_NOT_FOUND = "File not found"; + public static final String ERR_FUNCTION_NOT_FOUND = "Function not found"; + public static final String ERR_REQUIRED_A = "Geometry A may be required"; + public static final String ERR_REQUIRED_B = "Geometry B is required"; + public static final String ERR_WRONG_ARG_COUNT = "Arguments and parameters do not match"; + public static final String ERR_FUNCTION_ERR = "Error executing function"; + static final String[] helpDoc = new String[] { "", "Usage: jtsop - CLI for JTS operations", @@ -101,41 +108,38 @@ public static void main(String[] args) { JTSOpCmd cmd = new JTSOpCmd(); try { - CmdArgs cmdArgs = parseArgs(args); - - if (args.length == 0 || isHelp) { - printHelp(isHelp); - System.exit(0); - } - + CmdArgs cmdArgs = cmd.parseArgs(args); cmd.execute(cmdArgs); - } catch (Exception e) { + } + catch (CommandError e) { + // for command errors, just print the message + System.err.println(e.getMessage() ); + } + catch (Exception e) { + // unexpected errors get a stack track to help debugging e.printStackTrace(); } } - private static void printHelp(boolean showFunctions) { + private void printHelp(boolean showFunctions) { for (String s : helpDoc) { - System.out.println(s); + out.println(s); } if (showFunctions) { - System.out.println(); - System.out.println("Operations:"); + out.println(); + out.println("Operations:"); printFunctions(); } } - private static void printFunctions() { + private void printFunctions() { //TODO: include any loaded functions DoubleKeyMap funcMap = funcRegistry.getCategorizedGeometryFunctions(); - Collection categories = funcMap.keySet(); - for (Iterator i = categories.iterator(); i.hasNext();) { - String category = (String) i.next(); - - Collection funcs = funcMap.values(category); - for (Iterator j = funcs.iterator(); j.hasNext();) { - GeometryFunction fun = (GeometryFunction) j.next(); - System.out.println(category + "." + functionDesc(fun)); + Collection categories = funcMap.keySet(); + for (String category : categories) { + Collection funcs = funcMap.values(category); + for (GeometryFunction fun : funcs) { + out.println(category + "." + functionDesc(fun)); } } } @@ -147,29 +151,41 @@ private static String functionDesc(GeometryFunction fun) { //return geomFun.getName(); } - private static GeometryFactory geomFactory = new GeometryFactory(); + private GeometryFactory geomFactory = new GeometryFactory(); - private static GeometryFunctionRegistry funcRegistry = GeometryFunctionRegistry.createTestBuilderRegistry(); - private static CommandLine commandLine = createCmdLine(); + private GeometryFunctionRegistry funcRegistry = GeometryFunctionRegistry.createTestBuilderRegistry(); + private CommandLine commandLine = createCmdLine(); + + private boolean isHelp = false; + private boolean isHelpWithFunctions = false; + private boolean isVerbose = false; - private static boolean isHelp = false; - private static boolean isVerbose = false; + private CommandOutput out = new CommandOutput(); - private static class CmdArgs { + static class CmdArgs { String operation; String geomA; public String geomB; public String arg1; - String format = null; } - public JTSOpCmd() { } - private void execute(CmdArgs cmdArgs) throws IOException, Exception { + public void captureOutput() { + out = new CommandOutput(true); + } + + public String getOutput() { + return out.getOutput(); + } + void execute(CmdArgs cmdArgs) throws IOException, Exception { + if (isHelp || isHelpWithFunctions) { + printHelp(isHelpWithFunctions); + return; + } Geometry geomA = null; Geometry geomB = null; @@ -203,31 +219,43 @@ private void execute(CmdArgs cmdArgs) throws IOException, Exception { private Object executeFunction(CmdArgs cmdArgs, Geometry geomA, Geometry geomB) { GeometryFunction func = getFunction(cmdArgs.operation); if (func == null) { - System.err.println("Function not found: " + cmdArgs.operation); - return null; + throw new CommandError(ERR_FUNCTION_NOT_FOUND, cmdArgs.operation); } + checkFunctionArgs(func, geomB, cmdArgs.arg1); Object funArgs[] = createFunctionArgs(func, geomB, cmdArgs.arg1); if (isVerbose) { - System.out.println(writeOpSummary(cmdArgs)); + out.println(writeOpSummary(cmdArgs)); } Stopwatch timer = new Stopwatch(); Object result = null; try { result = func.invoke(geomA, funArgs); - } finally { + } + catch (NullPointerException ex) { + if (geomA == null) + throw new CommandError(ERR_REQUIRED_A, cmdArgs.operation); + // if A is present then must be something else + throw new CommandError(ERR_FUNCTION_ERR, ex.getMessage()); + } + finally { timer.stop(); } if (isVerbose) { - System.out.println("Time: " + timer.getTimeString()); + out.println("Time: " + timer.getTimeString()); } return result; } private Geometry readGeometry(String arg) throws Exception, IOException { if (isFilename(arg)) { - return IOUtil.readFile(arg ,geomFactory ); + try { + return IOUtil.readFile(arg ,geomFactory ); + } + catch (FileNotFoundException ex) { + throw new CommandError(ERR_FILE_NOT_FOUND, arg); + } } MultiFormatReader rdr = new MultiFormatReader(geomFactory); return rdr.read(arg); @@ -269,7 +297,7 @@ private void printResult(Object result, String outputFormat) { printGeometry((Geometry) result, outputFormat); return; } - System.out.println(result); + out.println(result); } private void printGeometry(Geometry geom, String outputFormat) { @@ -292,7 +320,7 @@ else if (outputFormat.equalsIgnoreCase(FORMAT_SVG)) { } if (txt == null) return; - System.out.println(txt); + out.println(txt); } private String writeGeoJSON(Geometry geom) { @@ -305,7 +333,7 @@ private void printGeometrySummary(String label, Geometry geom, String arg) { if (! isVerbose) return; String filename = ""; if (arg != null & isFilename(arg)) filename = " -- " + arg; - System.out.println( writeGeometrySummary(label, geom) + filename); + out.println( writeGeometrySummary(label, geom) + filename); } private String writeGeometrySummary(String label, @@ -318,6 +346,30 @@ private String writeGeometrySummary(String label, buf.append(" " + GeometryUtil.metricsSummary(g)); return buf.toString(); } + + private void checkFunctionArgs(GeometryFunction func, Geometry geomB, String arg1) { + Class[] paramTypes = func.getParameterTypes(); + int nParam = paramTypes.length; + + if (func.isBinary() && geomB == null) + throw new CommandError(ERR_REQUIRED_B); + /** + // MD not sure whether to check this? + if (! func.isBinary() && geomB != null) + throw new CommandError(ERR_REQUIRED_B); + */ + + /* + * check count of supplied args. + * Assumes B has been checked. + */ + int argCount = 0; + if (func.isBinary() && geomB != null) argCount++; + if (arg1 != null) argCount++; + if (nParam != argCount) { + throw new CommandError(ERR_WRONG_ARG_COUNT, func.getName()); + } + } private Object[] createFunctionArgs(GeometryFunction func, Geometry geomB, String arg1) { Class[] paramTypes = func.getParameterTypes(); @@ -328,6 +380,7 @@ private Object[] createFunctionArgs(GeometryFunction func, Geometry geomB, Strin paramVal[0] = geomB; iparam++; } + // just handling one scalar arg for now if (iparam < paramVal.length) { paramVal[iparam] = SwingUtil.coerce(arg1, paramTypes[iparam]); } @@ -345,20 +398,25 @@ private GeometryFunction getFunction(String operation) { return funcRegistry.find(category, name); } - private static CmdArgs parseArgs(String[] args) throws ParseException, ClassNotFoundException { - commandLine.parse(args); - + CmdArgs parseArgs(String[] args) throws ParseException, ClassNotFoundException { CmdArgs cmdArgs = new CmdArgs(); + if (args.length == 0) { + isHelp = true; + return cmdArgs; + } + + commandLine.parse(args); + if (commandLine.hasOption(CommandOptions.GEOMFUNC)) { Option opt = commandLine.getOption(CommandOptions.GEOMFUNC); for (int i = 0; i < opt.getNumArgs(); i++) { String geomFuncClassname = opt.getArg(i); try { funcRegistry.add(geomFuncClassname); - System.out.println("Added Geometry Functions from: " + geomFuncClassname); + out.println("Added Geometry Functions from: " + geomFuncClassname); } catch (ClassNotFoundException ex) { - System.out.println("Unable to load function class: " + geomFuncClassname); + out.println("Unable to load function class: " + geomFuncClassname); } } } @@ -370,7 +428,7 @@ private static CmdArgs parseArgs(String[] args) throws ParseException, ClassNotF cmdArgs.format = commandLine.getOptionArg(CommandOptions.FORMAT, 0); isVerbose = commandLine.hasOption(CommandOptions.VERBOSE) || commandLine.hasOption(CommandOptions.V); - isHelp = commandLine.hasOption(CommandOptions.HELP); + isHelpWithFunctions = commandLine.hasOption(CommandOptions.HELP); String[] freeArgs = commandLine.getOptionArgs(OptionSpec.OPTION_FREE_ARGS); if (freeArgs != null) { @@ -385,7 +443,7 @@ private static CmdArgs parseArgs(String[] args) throws ParseException, ClassNotF return cmdArgs; } - private static CommandLine createCmdLine() { + private CommandLine createCmdLine() { commandLine = new CommandLine('-'); commandLine.addOptionSpec(new OptionSpec(CommandOptions.GEOMFUNC, OptionSpec.NARGS_ONE_OR_MORE)) .addOptionSpec(new OptionSpec(CommandOptions.VERBOSE, 0)) diff --git a/modules/app/src/test/java/org/locationtech/jtstest/cmd/JTSOpCmdTest.java b/modules/app/src/test/java/org/locationtech/jtstest/cmd/JTSOpCmdTest.java new file mode 100644 index 0000000000..cc88ddac54 --- /dev/null +++ b/modules/app/src/test/java/org/locationtech/jtstest/cmd/JTSOpCmdTest.java @@ -0,0 +1,138 @@ +package org.locationtech.jtstest.cmd; + +import junit.framework.TestCase; + +public class JTSOpCmdTest extends TestCase { + private boolean isVerbose = true; + + public JTSOpCmdTest(String Name_) { + super(Name_); + } + + public static void main(String[] args) { + String[] testCaseName = {JTSOpCmdTest.class.getName()}; + junit.textui.TestRunner.main(testCaseName); + } + + public void testHelp() { + runCmd( args("-help"), "Usage"); + } + + public void testErrorFileNotFoundA() { + runCmdError( args("-a", "missing.wkt"), + JTSOpCmd.ERR_FILE_NOT_FOUND ); + } + + public void testErrorFileNotFoundB() { + runCmdError( args("-b", "missing.wkt"), + JTSOpCmd.ERR_FILE_NOT_FOUND ); + } + + public void testErrorFunctioNotFound() { + runCmdError( args("-a", "POINT ( 1 1 )", "buffer" ), + JTSOpCmd.ERR_FUNCTION_NOT_FOUND ); + } + + public void testErrorMissingArgBuffer() { + runCmdError( args("-a", "POINT ( 1 1 )", "Buffer.buffer" ), + JTSOpCmd.ERR_WRONG_ARG_COUNT ); + } + + public void testErrorMissingGeomABuffer() { + runCmdError( args("Buffer.buffer", "10" ), + JTSOpCmd.ERR_REQUIRED_A ); + } + + public void testErrorMissingGeomBUnion() { + runCmdError( args("-a", "POINT ( 1 1 )", "Overlay.union" ), + JTSOpCmd.ERR_REQUIRED_B ); + } + + public void testErrorMissingGeomAUnion() { + runCmdError( args("-b", "POINT ( 1 1 )", "Overlay.union" ), + JTSOpCmd.ERR_REQUIRED_A ); + } + + public void testOpEnvelope() { + runCmd( args("-a", "LINESTRING ( 1 1, 2 2)", "-f", "wkt", "envelope"), + "POLYGON" ); + } + + public void testOpLength() { + runCmd( args("-a", "LINESTRING ( 1 0, 2 0 )", "-f", "txt", "length"), + "1" ); + } + + public void testOpUnionLines() { + runCmd( args("-a", "LINESTRING ( 1 0, 2 0 )", "-b", "LINESTRING ( 2 0, 3 0 )", "-f", "wkt", "Overlay.union"), + "MULTILINESTRING ((1 0, 2 0), (2 0, 3 0))" ); + } + + public void testFormatWKB() { + runCmd( args("-a", "LINESTRING ( 1 1, 2 2)", "-f", "wkb"), + "0000000002000000023FF00000000000003FF000000000000040000000000000004000000000000000" ); + } + + public void testFormatGeoJSON() { + runCmd( args("-a", "LINESTRING ( 1 1, 2 2)", "-f", "geojson"), + "{\"type\":\"LineString\",\"coordinates\":[[1,1],[2,2]]}" ); + } + + public void testFormatSVG() { + runCmd( args("-a", "LINESTRING ( 1 1, 2 2)", "-f", "svg"), + "" ); + } + + private String[] args(String ... args) { + return args; + } + + public void runCmd(String[] args, String expected) + { + JTSOpCmd cmd = new JTSOpCmd(); + cmd.captureOutput(); + try { + JTSOpCmd.CmdArgs cmdArgs = cmd.parseArgs(args); + cmd.execute(cmdArgs); + } catch (Exception e) { + e.printStackTrace(); + } + checkExpected( cmd.getOutput(), expected); + } + + public void runCmdError(String[] args) { + runCmdError(args, null); + } + + + public void runCmdError(String[] args, String expected) + { + JTSOpCmd cmd = new JTSOpCmd(); + try { + JTSOpCmd.CmdArgs cmdArgs = cmd.parseArgs(args); + cmd.execute(cmdArgs); + } + catch (CommandError e) { + if (expected != null) checkExpected( e.getMessage(), expected ); + return; + } + catch (Exception e) { + e.printStackTrace(); + } + assertTrue("Expected error but command completed successfully", false); + } + + private void checkExpected(String actual, String expected) { + boolean found = actual.contains(expected); + if (isVerbose && ! found) { + System.out.println("Expected: " + expected); + System.out.println("Actual: " + actual); + } + assertTrue( found ); + } +}