From ff888d239b7ab1d2109f855bc862b11ea510db2d Mon Sep 17 00:00:00 2001 From: Owen Stephens Date: Sun, 17 Mar 2019 13:11:26 +0000 Subject: [PATCH 1/4] Added new exception class --- .../refine/util/GetProjectIDException.java | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 main/src/com/google/refine/util/GetProjectIDException.java diff --git a/main/src/com/google/refine/util/GetProjectIDException.java b/main/src/com/google/refine/util/GetProjectIDException.java new file mode 100644 index 000000000..68888e6e6 --- /dev/null +++ b/main/src/com/google/refine/util/GetProjectIDException.java @@ -0,0 +1,53 @@ +/* + +Copyright 2019, Owen Stephens +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the following disclaimer +in the documentation and/or other materials provided with the +distribution. + * The name of the contributor may not be used to endorse or promote products +derived from this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +*/ + +package com.google.refine.util; + +/** + * Thrown when a unique project ID cannot be found based on a project name. + */ +public class GetProjectIDException extends Exception { + + private static final long serialVersionUID = 1552825467L; + + /** + * Default GetProjectID format exception. + */ + public GetProjectIDException() { super(); } + + /** + * GetProjectID exception. + * + * @param message error message + */ + public GetProjectIDException(String message) { super(message); } +} From ae5f72a8df5bd1109b5f70fd833f570396b782e0 Mon Sep 17 00:00:00 2001 From: Owen Stephens Date: Sun, 17 Mar 2019 13:14:58 +0000 Subject: [PATCH 2/4] Refactor cross function to be more robust & improve diagnostics on fail --- .../com/google/refine/InterProjectModel.java | 6 +++--- .../src/com/google/refine/ProjectManager.java | 20 +++++++++++++++---- .../google/refine/expr/functions/Cross.java | 19 ++++++++++++++---- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/main/src/com/google/refine/InterProjectModel.java b/main/src/com/google/refine/InterProjectModel.java index 295042612..eb69aec47 100644 --- a/main/src/com/google/refine/InterProjectModel.java +++ b/main/src/com/google/refine/InterProjectModel.java @@ -97,13 +97,13 @@ public class InterProjectModel { * @param toColumn * @return */ - public ProjectJoin getJoin(String fromProject, String fromColumn, String toProject, String toColumn) { + public ProjectJoin getJoin(Long fromProject, String fromColumn, Long toProject, String toColumn) { String key = fromProject + ";" + fromColumn + ";" + toProject + ";" + toColumn; if (!_joins.containsKey(key)) { ProjectJoin join = new ProjectJoin( - ProjectManager.singleton.getProjectID(fromProject), + fromProject, fromColumn, - ProjectManager.singleton.getProjectID(toProject), + toProject, toColumn ); diff --git a/main/src/com/google/refine/ProjectManager.java b/main/src/com/google/refine/ProjectManager.java index 66f2e6dcd..89070ac29 100644 --- a/main/src/com/google/refine/ProjectManager.java +++ b/main/src/com/google/refine/ProjectManager.java @@ -33,6 +33,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. package com.google.refine; +import com.google.refine.util.GetProjectIDException; import java.io.IOException; import java.io.InputStream; import java.time.LocalDateTime; @@ -383,15 +384,26 @@ public abstract class ProjectManager { * @param name * The name of the project * @return - * The id of the project, or -1 if it cannot be found + * The id of the project + * @throws GetProjectIDException + * If no unique project is found with the given name */ - public long getProjectID(String name) { + public long getProjectID(String name) throws GetProjectIDException { + Integer c = 0; + Long id = 0L; for (Entry entry : _projectsMetadata.entrySet()) { if (entry.getValue().getName().equals(name)) { - return entry.getKey(); + id = entry.getKey(); + c += 1; } } - return -1; + if (c == 1) { + return id; + } else if (c == 0) { + throw new GetProjectIDException("Unable to find project with name: " + name); + } else { + throw new GetProjectIDException(c + " projects found with name: " + name); + } } /** diff --git a/main/src/com/google/refine/expr/functions/Cross.java b/main/src/com/google/refine/expr/functions/Cross.java index b9ed764f2..07ecee5da 100644 --- a/main/src/com/google/refine/expr/functions/Cross.java +++ b/main/src/com/google/refine/expr/functions/Cross.java @@ -33,6 +33,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. package com.google.refine.expr.functions; +import com.google.refine.util.GetProjectIDException; import java.util.Properties; import com.google.refine.InterProjectModel.ProjectJoin; @@ -51,17 +52,27 @@ public class Cross implements Function { // 1st argument can take either value or cell(for backward compatibility) Object v = args[0]; Object toProjectName = args[1]; - Object toColumnName = args[2]; + Object toColumnName = args[2]; + Long toProjectID; if (v != null && ( v instanceof String || v instanceof WrappedCell ) && toProjectName != null && toProjectName instanceof String && toColumnName != null && toColumnName instanceof String) { - + try { + toProjectID = ProjectManager.singleton.getProjectID((String) toProjectName); + } catch (GetProjectIDException e){ + return new EvalError(e.getMessage()); + } ProjectJoin join = ProjectManager.singleton.getInterProjectModel().getJoin( - ProjectManager.singleton.getProjectMetadata(((Project) bindings.get("project")).id).getName(), + // getJoin(Long fromProject, String fromColumn, Long toProject, String toColumn) { + // source project name + (Long) ((Project) bindings.get("project")).id, + // source column name (String) bindings.get("columnName"), - (String) toProjectName, + // target project name + toProjectID, + // target column name (String) toColumnName ); if(v instanceof String) { From 3e01c15c373711498a24362b68b265794d438acd Mon Sep 17 00:00:00 2001 From: Owen Stephens Date: Sat, 23 Mar 2019 14:15:00 +0000 Subject: [PATCH 3/4] Throw JoinException if attempt to join with non-existent column --- .../com/google/refine/InterProjectModel.java | 16 ++++-- .../google/refine/expr/functions/Cross.java | 33 +++++++----- .../com/google/refine/util/JoinException.java | 53 +++++++++++++++++++ 3 files changed, 85 insertions(+), 17 deletions(-) create mode 100644 main/src/com/google/refine/util/JoinException.java diff --git a/main/src/com/google/refine/InterProjectModel.java b/main/src/com/google/refine/InterProjectModel.java index eb69aec47..f911036ca 100644 --- a/main/src/com/google/refine/InterProjectModel.java +++ b/main/src/com/google/refine/InterProjectModel.java @@ -46,6 +46,7 @@ import com.google.refine.expr.WrappedRow; import com.google.refine.model.Column; import com.google.refine.model.Project; import com.google.refine.model.Row; +import com.google.refine.util.JoinException; public class InterProjectModel { static public class ProjectJoin { @@ -97,7 +98,7 @@ public class InterProjectModel { * @param toColumn * @return */ - public ProjectJoin getJoin(Long fromProject, String fromColumn, Long toProject, String toColumn) { + public ProjectJoin getJoin(Long fromProject, String fromColumn, Long toProject, String toColumn) throws JoinException { String key = fromProject + ";" + fromColumn + ";" + toProject + ";" + toColumn; if (!_joins.containsKey(key)) { ProjectJoin join = new ProjectJoin( @@ -142,21 +143,28 @@ public class InterProjectModel { } } - protected void computeJoin(ProjectJoin join) { + protected void computeJoin(ProjectJoin join) throws JoinException { if (join.fromProjectID < 0 || join.toProjectID < 0) { return; } Project fromProject = ProjectManager.singleton.getProject(join.fromProjectID); + ProjectMetadata fromProjectMD = ProjectManager.singleton.getProjectMetadata(join.fromProjectID); Project toProject = ProjectManager.singleton.getProject(join.toProjectID); + ProjectMetadata toProjectMD = ProjectManager.singleton.getProjectMetadata(join.toProjectID); + + // split this test to check each one and throw an appropriate error if (fromProject == null || toProject == null) { return; } Column fromColumn = fromProject.columnModel.getColumnByName(join.fromProjectColumnName); Column toColumn = toProject.columnModel.getColumnByName(join.toProjectColumnName); - if (fromColumn == null || toColumn == null) { - return; + if (fromColumn == null) { + throw new JoinException("Unable to find column " + join.fromProjectColumnName + " in project " + fromProjectMD.getName()); + } + if (toColumn == null) { + throw new JoinException("Unable to find column " + join.toProjectColumnName + " in project " + toProjectMD.getName()); } for (Row fromRow : fromProject.rows) { diff --git a/main/src/com/google/refine/expr/functions/Cross.java b/main/src/com/google/refine/expr/functions/Cross.java index 07ecee5da..c7da0c4d8 100644 --- a/main/src/com/google/refine/expr/functions/Cross.java +++ b/main/src/com/google/refine/expr/functions/Cross.java @@ -33,7 +33,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. package com.google.refine.expr.functions; -import com.google.refine.util.GetProjectIDException; import java.util.Properties; import com.google.refine.InterProjectModel.ProjectJoin; @@ -43,6 +42,8 @@ import com.google.refine.expr.WrappedCell; import com.google.refine.grel.ControlFunctionRegistry; import com.google.refine.grel.Function; import com.google.refine.model.Project; +import com.google.refine.util.GetProjectIDException; +import com.google.refine.util.JoinException; public class Cross implements Function { @@ -53,7 +54,8 @@ public class Cross implements Function { Object v = args[0]; Object toProjectName = args[1]; Object toColumnName = args[2]; - Long toProjectID; + Long toProjectID; + ProjectJoin join; if (v != null && ( v instanceof String || v instanceof WrappedCell ) && @@ -64,17 +66,22 @@ public class Cross implements Function { } catch (GetProjectIDException e){ return new EvalError(e.getMessage()); } - ProjectJoin join = ProjectManager.singleton.getInterProjectModel().getJoin( - // getJoin(Long fromProject, String fromColumn, Long toProject, String toColumn) { - // source project name - (Long) ((Project) bindings.get("project")).id, - // source column name - (String) bindings.get("columnName"), - // target project name - toProjectID, - // target column name - (String) toColumnName - ); + // add a try/catch here - error should bubble up from getInterProjectModel.computeJoin once that's modified + try { + join = ProjectManager.singleton.getInterProjectModel().getJoin( + // getJoin(Long fromProject, String fromColumn, Long toProject, String toColumn) { + // source project name + (Long) ((Project) bindings.get("project")).id, + // source column name + (String) bindings.get("columnName"), + // target project name + toProjectID, + // target column name + (String) toColumnName + ); + } catch (JoinException e) { + return new EvalError(e.getMessage()); + } if(v instanceof String) { return join.getRows(v); } else { diff --git a/main/src/com/google/refine/util/JoinException.java b/main/src/com/google/refine/util/JoinException.java new file mode 100644 index 000000000..40d7ddd0b --- /dev/null +++ b/main/src/com/google/refine/util/JoinException.java @@ -0,0 +1,53 @@ +/* + +Copyright 2019, Owen Stephens +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the following disclaimer +in the documentation and/or other materials provided with the +distribution. + * The name of the contributor may not be used to endorse or promote products +derived from this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +*/ + +package com.google.refine.util; + +/** + * Thrown when there is an error on a project Join. + */ +public class JoinException extends Exception { + + private static final long serialVersionUID = 1553348086L; + + /** + * Default JoinException format exception. + */ + public JoinException() { super(); } + + /** + * JoinException exception. + * + * @param message error message + */ + public JoinException(String message) { super(message); } +} From 722db56071e202c0d434180fff80ceeb363e0889 Mon Sep 17 00:00:00 2001 From: Owen Stephens Date: Sat, 23 Mar 2019 14:15:23 +0000 Subject: [PATCH 4/4] Add tests for new Cross errors --- .../expr/functions/CrossFunctionTests.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/main/tests/server/src/com/google/refine/tests/expr/functions/CrossFunctionTests.java b/main/tests/server/src/com/google/refine/tests/expr/functions/CrossFunctionTests.java index 289df274c..7f89844fd 100644 --- a/main/tests/server/src/com/google/refine/tests/expr/functions/CrossFunctionTests.java +++ b/main/tests/server/src/com/google/refine/tests/expr/functions/CrossFunctionTests.java @@ -12,6 +12,7 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.BeforeTest; import org.testng.annotations.Test; +import com.google.refine.ProjectManager; import com.google.refine.expr.EvalError; import com.google.refine.expr.HasFieldsListImpl; import com.google.refine.expr.WrappedRow; @@ -22,6 +23,7 @@ import com.google.refine.model.Project; import com.google.refine.model.Row; import com.google.refine.model.Cell; import com.google.refine.tests.RefineTest; +import com.google.refine.util.GetProjectIDException; /** * Test cases for cross function. @@ -39,6 +41,8 @@ public class CrossFunctionTests extends RefineTest { // dependencies Project projectGift; Project projectAddress; + Project projectDuplicate1; + Project projectDuplicate2; // data from: https://github.com/OpenRefine/OpenRefine/wiki/GREL-Other-Functions @BeforeMethod @@ -64,6 +68,11 @@ public class CrossFunctionTests extends RefineTest { + "integer,1600\n" + "boolean,true\n"; projectGift = createCSVProject(projectName, input); + projectName = "Duplicate"; + input = "Col1,Col2"; + projectDuplicate1 = createCSVProject(projectName, input); + projectDuplicate2 = createCSVProject(projectName, input); + bindings.put("project", projectGift); @@ -79,6 +88,28 @@ public class CrossFunctionTests extends RefineTest { bindings.put("columnName", "recipient"); } + @Test + public void crossFunctionMissingProject() throws Exception { + String nonExistentProject = "NOPROJECT"; + Assert.assertEquals(((EvalError) invoke("cross", "Anne", nonExistentProject, "friend")).message, + "Unable to find project with name: " + nonExistentProject); + } + + @Test + public void crossFunctionMultipleProjects() throws Exception { + String duplicateProjectName = "Duplicate"; + Assert.assertEquals(((EvalError) invoke("cross", "Anne", duplicateProjectName, "friend")).message, + "2 projects found with name: " + duplicateProjectName); + } + + @Test + public void crossFunctionMissingColumn() throws Exception { + String nonExistentColumn = "NoColumn"; + String projectName = "My Address Book"; + Assert.assertEquals(((EvalError) invoke("cross", "mary", projectName, nonExistentColumn)).message, + "Unable to find column " + nonExistentColumn + " in project " + projectName); + } + @Test public void crossFunctionOneToOneTest() throws Exception { Row row = ((Row)((WrappedRow) ((HasFieldsListImpl) invoke("cross", "mary", "My Address Book", "friend")).get(0)).row);