From 7d9018f24777512ca76d6e3025d845e007161469 Mon Sep 17 00:00:00 2001 From: Becky Jackson Date: Tue, 18 May 2021 14:05:55 -0700 Subject: [PATCH] Fix entity rendering in report when value is an entity (#874) * Fix entity rendering in report when value is an entity * Return null on IRIs with spaces --- CHANGELOG.md | 3 + .../java/org/obolibrary/robot/IOHelper.java | 5 + .../org/obolibrary/robot/ReportOperation.java | 9 +- .../org/obolibrary/robot/checks/Report.java | 163 ++++++++++-------- .../obolibrary/robot/checks/Violation.java | 67 ++++++- 5 files changed, 160 insertions(+), 87 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15f0c270f..66574e9e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix missing annotations from [`export`] [#850] - Fail on unknown rule names in [`report`] [#858] - Fix behaviour of `--preserve-structure` when using internal or external axiom selectors for [`remove`] or [`filter`] [#816] +- Fix value rendering for entities in [`report`] [#874] ## [1.8.1] - 2021-01-27 @@ -252,6 +253,8 @@ First official release of ROBOT! [`template`]: http://robot.obolibrary.org/template [`validate`]: http://robot.obolibrary.org/validate +[#874]: https://github.com/ontodev/robot/pull/874 +[#858]: https://github.com/ontodev/robot/pull/858 [#850]: https://github.com/ontodev/robot/pull/850 [#834]: https://github.com/ontodev/robot/pull/834 [#823]: https://github.com/ontodev/robot/pull/823 diff --git a/robot-core/src/main/java/org/obolibrary/robot/IOHelper.java b/robot-core/src/main/java/org/obolibrary/robot/IOHelper.java index 7f93f2d5b..dd5ab85bf 100644 --- a/robot-core/src/main/java/org/obolibrary/robot/IOHelper.java +++ b/robot-core/src/main/java/org/obolibrary/robot/IOHelper.java @@ -926,6 +926,11 @@ public IRI createIRI(String term) { logger.warn(e.getMessage()); return null; } + + if (iri.toString().contains(" ")) { + // Invalid IRI + return null; + } return iri; } diff --git a/robot-core/src/main/java/org/obolibrary/robot/ReportOperation.java b/robot-core/src/main/java/org/obolibrary/robot/ReportOperation.java index 36c4cbd1c..2ff5ac01d 100644 --- a/robot-core/src/main/java/org/obolibrary/robot/ReportOperation.java +++ b/robot-core/src/main/java/org/obolibrary/robot/ReportOperation.java @@ -785,7 +785,7 @@ private static Map getProfile(String path) throws IOException { if (path == null) { is = ReportOperation.class.getResourceAsStream("/report_profile.txt"); } else { - is = new FileInputStream(new File(path)); + is = new FileInputStream(path); } try (BufferedReader br = new BufferedReader(new InputStreamReader(is))) { String line; @@ -945,12 +945,13 @@ private static List getViolationsFromResults( if (value != null) { IRI valIRI = ioHelper.createIRI(value); if (valIRI != null) { - violation.addStatement(e, valIRI); + OWLEntity v = dataFactory.getOWLClass(valIRI); + violation.addStatement(e, v); } else { - violation.addStatement(e, dataFactory.getOWLLiteral(value)); + violation.addStatement(e, value); } } else { - violation.addStatement(e, null); + violation.addStatement(e, ""); } } violations.add(violation); diff --git a/robot-core/src/main/java/org/obolibrary/robot/checks/Report.java b/robot-core/src/main/java/org/obolibrary/robot/checks/Report.java index 650136129..b58e2f069 100644 --- a/robot-core/src/main/java/org/obolibrary/robot/checks/Report.java +++ b/robot-core/src/main/java/org/obolibrary/robot/checks/Report.java @@ -57,7 +57,7 @@ public class Report { private Integer errorCount = 0; /** IOHelper to use. */ - private IOHelper ioHelper; + private final IOHelper ioHelper; /** Manager to use. */ private OWLOntologyManager manager; @@ -270,20 +270,7 @@ public Integer getViolationCount(String ruleName) throws Exception { * @return export Table object */ public Table toTable(String format) { - CURIEShortFormProvider curieProvider = new CURIEShortFormProvider(ioHelper.getPrefixes()); - QuotedAnnotationValueShortFormProvider nameProvider = - new QuotedAnnotationValueShortFormProvider( - manager, - curieProvider, - ioHelper.getPrefixManager(), - Collections.singletonList(OWLManager.getOWLDataFactory().getRDFSLabel()), - Collections.emptyMap()); - ShortFormProvider provider; - if (useLabels) { - provider = nameProvider; - } else { - provider = curieProvider; - } + ShortFormProvider provider = getProvider(); Table table = new Table(format); for (String h : header) { @@ -316,20 +303,7 @@ public String toJSON() throws IOException { * @return YAML string */ public String toYAML() { - CURIEShortFormProvider curieProvider = new CURIEShortFormProvider(ioHelper.getPrefixes()); - QuotedAnnotationValueShortFormProvider nameProvider = - new QuotedAnnotationValueShortFormProvider( - manager, - curieProvider, - ioHelper.getPrefixManager(), - Collections.singletonList(OWLManager.getOWLDataFactory().getRDFSLabel()), - Collections.emptyMap()); - ShortFormProvider provider; - if (useLabels) { - provider = nameProvider; - } else { - provider = curieProvider; - } + ShortFormProvider provider = getProvider(); return yamlHelper(provider, ERROR, error) + yamlHelper(provider, WARN, warn) + yamlHelper(provider, INFO, info); @@ -372,7 +346,7 @@ private void addToTable( } } Cell subjectCell = new Cell(columns.get(2), subject); - for (Entry> statement : v.entityStatements.entrySet()) { + for (Entry> statement : v.entityStatements.entrySet()) { // Property of the violation for the following rows String property = ""; if (statement.getKey() != null) { @@ -382,33 +356,43 @@ private void addToTable( Cell propertyCell = new Cell(columns.get(3), property); if (statement.getValue().isEmpty()) { - Row row = new Row(level); Cell valueCell = new Cell(columns.get(4), ""); - row.add(levelCell); - row.add(ruleCell); - row.add(subjectCell); - row.add(propertyCell); - row.add(valueCell); - table.addRow(row); + addRowToTable(table, level, levelCell, ruleCell, subjectCell, propertyCell, valueCell); } else { - for (OWLObject o : statement.getValue()) { - Row row = new Row(level); + for (OWLEntity e : statement.getValue()) { + String value = OntologyHelper.renderManchester(e, provider, displayRenderer); + Cell valueCell = new Cell(columns.get(4), value); + addRowToTable( + table, level, levelCell, ruleCell, subjectCell, propertyCell, valueCell); + } + } + } - String value = ""; - if (o != null) { - value = OntologyHelper.renderManchester(o, provider, displayRenderer); - } + for (Entry> statement : v.literalStatements.entrySet()) { + // Property of the violation for the following rows + String property = ""; + if (statement.getKey() != null) { + property = + OntologyHelper.renderManchester(statement.getKey(), provider, displayRenderer); + } + Cell propertyCell = new Cell(columns.get(3), property); + if (statement.getValue().isEmpty()) { + Cell valueCell = new Cell(columns.get(4), ""); + addRowToTable(table, level, levelCell, ruleCell, subjectCell, propertyCell, valueCell); + } else { + for (String value : statement.getValue()) { + if (value == null) { + value = ""; + } Cell valueCell = new Cell(columns.get(4), value); - row.add(levelCell); - row.add(ruleCell); - row.add(subjectCell); - row.add(propertyCell); - row.add(valueCell); - table.addRow(row); + addRowToTable( + table, level, levelCell, ruleCell, subjectCell, propertyCell, valueCell); } } } + + // Support for old statements method for (Entry> statement : v.statements.entrySet()) { String property = statement.getKey(); if (property == null) { @@ -417,27 +401,16 @@ private void addToTable( Cell propertyCell = new Cell(columns.get(3), property); if (statement.getValue().isEmpty()) { - Row row = new Row(level); Cell valueCell = new Cell(columns.get(4), ""); - row.add(levelCell); - row.add(ruleCell); - row.add(subjectCell); - row.add(propertyCell); - row.add(valueCell); - table.addRow(row); + addRowToTable(table, level, levelCell, ruleCell, subjectCell, propertyCell, valueCell); } else { for (String value : statement.getValue()) { if (value == null) { continue; } - Row row = new Row(level); Cell valueCell = new Cell(columns.get(4), value); - row.add(levelCell); - row.add(ruleCell); - row.add(subjectCell); - row.add(propertyCell); - row.add(valueCell); - table.addRow(row); + addRowToTable( + table, level, levelCell, ruleCell, subjectCell, propertyCell, valueCell); } } } @@ -445,6 +418,54 @@ private void addToTable( } } + /** + * Add a row to a table. + * + * @param table Table to add row to + * @param level String violation level + * @param levelCell Cell for level + * @param ruleCell Cell for rule + * @param subjectCell Cell for subject + * @param propertyCell Cell for property + * @param valueCell Cell for value + */ + private void addRowToTable( + Table table, + String level, + Cell levelCell, + Cell ruleCell, + Cell subjectCell, + Cell propertyCell, + Cell valueCell) { + Row row = new Row(level); + row.add(levelCell); + row.add(ruleCell); + row.add(subjectCell); + row.add(propertyCell); + row.add(valueCell); + table.addRow(row); + } + + /** + * Get a ShortFormProvider to render entities based on if we are rendering labels or not. + * + * @return ShortFormProvider that either uses names (labels or CURIEs) or CURIEs + */ + private ShortFormProvider getProvider() { + CURIEShortFormProvider curieProvider = new CURIEShortFormProvider(ioHelper.getPrefixes()); + QuotedAnnotationValueShortFormProvider nameProvider = + new QuotedAnnotationValueShortFormProvider( + manager, + curieProvider, + ioHelper.getPrefixManager(), + Collections.singletonList(OWLManager.getOWLDataFactory().getRDFSLabel()), + Collections.emptyMap()); + if (useLabels) { + return nameProvider; + } + return curieProvider; + } + /** * Given a rule name, return a rule name. If the string starts with "file", the name is the path * and should be stripped to just the name. Otherwise, the input is returned. @@ -493,7 +514,7 @@ private String yamlHelper( OntologyHelper.renderManchester(v.entity, provider, RendererType.OBJECT_RENDERER); sb.append(" - subject: \"").append(subject).append("\""); sb.append("\n"); - for (Entry> statement : v.entityStatements.entrySet()) { + for (Entry> statement : v.entityStatements.entrySet()) { String property = OntologyHelper.renderManchester( statement.getKey(), provider, RendererType.OBJECT_RENDERER); @@ -504,7 +525,7 @@ private String yamlHelper( } sb.append(" values:"); sb.append("\n"); - for (OWLObject value : statement.getValue()) { + for (OWLEntity value : statement.getValue()) { String display = ""; if (value != null) { display = @@ -566,16 +587,10 @@ public Set getIRIs(Map> violationSets) { for (Entry> vs : violationSets.entrySet()) { for (Violation v : vs.getValue()) { iris.add(v.entity.getIRI().toString()); - for (Entry> statement : v.entityStatements.entrySet()) { + for (Entry> statement : v.entityStatements.entrySet()) { iris.add(statement.getKey().getIRI().toString()); - for (OWLObject value : statement.getValue()) { - if (value instanceof OWLEntity) { - OWLEntity e = (OWLEntity) value; - iris.add(e.getIRI().toString()); - } else if (value instanceof IRI) { - IRI iri = (IRI) value; - iris.add(iri.toString()); - } + for (OWLEntity value : statement.getValue()) { + iris.add(value.getIRI().toString()); } } } diff --git a/robot-core/src/main/java/org/obolibrary/robot/checks/Violation.java b/robot-core/src/main/java/org/obolibrary/robot/checks/Violation.java index ba077e3d7..de3f23135 100644 --- a/robot-core/src/main/java/org/obolibrary/robot/checks/Violation.java +++ b/robot-core/src/main/java/org/obolibrary/robot/checks/Violation.java @@ -13,8 +13,11 @@ public class Violation { public String subject; // Statements about OWLEntity subject - public Map> entityStatements = new HashMap<>(); + public Map> entityStatements = new HashMap<>(); // Statements about String subject + public Map> literalStatements = new HashMap<>(); + + // Deprecated statements with string properties public Map> statements = new HashMap<>(); /** @@ -27,13 +30,22 @@ public Violation(OWLEntity subject) { } /** - * Add a statement to the Violation about the subject. + * Create a new Violation object about a String. This is used for blank nodes. + * + * @param subject String subject + */ + public Violation(String subject) { + this.subject = subject; + } + + /** + * Add a statement with an OWLEntity value to the Violation about the subject. * * @param property OWLEntity property - * @param value OWLObject value + * @param value OWLEntity value */ - public void addStatement(OWLEntity property, OWLObject value) { - List values; + public void addStatement(OWLEntity property, OWLEntity value) { + List values; if (entityStatements.get(property) != null) { // This property already has one value in the map // Add this value to the existing list @@ -51,19 +63,38 @@ public void addStatement(OWLEntity property, OWLObject value) { entityStatements.put(property, values); } - /** @param subject String subject */ - public Violation(String subject) { - this.subject = subject; + /** + * Add a literal statement to the Violation about the subject. + * + * @param property OWLEntity property + * @param value String literal value + */ + public void addStatement(OWLEntity property, String value) { + List values; + if (literalStatements.get(property) != null) { + values = new ArrayList<>(literalStatements.get(property)); + values.add(value); + } else { + if (value != null) { + values = Collections.singletonList(value); + } else { + values = Collections.emptyList(); + } + } + literalStatements.put(property, values); } /** * @param property String property * @param value String value + * @deprecated String properties are no longer recommended; create an OWLEntity for the property + * instead */ + @Deprecated public void addStatement(String property, String value) { List values; if (statements.get(property) != null) { - values = new ArrayList(statements.get(property)); + values = new ArrayList<>(statements.get(property)); values.add(value); } else { if (value != null) { @@ -74,4 +105,22 @@ public void addStatement(String property, String value) { } statements.put(property, values); } + + /** + * @param property String property + * @param object OWLObject value + * @deprecated OWLObjects are no longer supported; use OWLEntity or String literal instead + */ + @Deprecated + public void addStatement(OWLEntity property, OWLObject object) { + logger.error("Do not add OWLObjects to Violations; use an OWLEntity or String literal instead"); + if (object instanceof OWLEntity) { + addStatement(property, (OWLEntity) object); + } else if (object instanceof OWLLiteral) { + OWLLiteral lit = (OWLLiteral) object; + addStatement(property, lit.getLiteral()); + } else { + addStatement(property, object.toString()); + } + } }