From acd8739754cd970db0909c69a55a5af4599e1e16 Mon Sep 17 00:00:00 2001 From: ignazio Date: Sun, 3 Mar 2019 20:42:15 +0000 Subject: [PATCH] Fix Cyclic imports and missing declarations cause parsing error #798 Ontology A imports ontology B, ontology B imports ontoloy A and ontology C, ontology C provides declaration for property P, ontology A uses property P. When loading A through its IRI, an ontology rename failure exception is raised. the reason is that A is parsed twice, once before and once after ontology C is parsed. However, when ontology C has yet to be parsed, there is no declaration available for P. This means that P is assumed to be an annotation property (only possible assumption, as the alternative is to throw a parsing error), and when the parsing of A completes, its id needs to be set with the value read from the ontology. However, at this point the two ontologies have the same id but not the same content - P has one guessed type and one parsed type, and the corresponding axioms differ. Hence, the api interprets this as two diffrent ontologies with the same id - which cannot coexist in the same manager. The full solution for this is for the parsers to work on two levels: ontology imports closure and entity declarations parsing, and axiom parsing. The ontologies to import and the declarations in each ontology would be parsed first, and once the closure has been computed, each ontology can be parsed with full knowledge of the entities declared - this removes the source of the error, in that there won't be any more cases of entities being used when their declaration has not been processed yet due to imports resolution and cycles. (This is how parsing is outlined in the specs, but it's not working exactly this way in practice and it's a considerable refactor for the current parsers.) (Other root issue: cycles in imports, this is not an indicator of a healthy ontology structure. It's allowed in OWL 2, but I cannot think of a scenario where the ontolgies wouldn't be easier to work with with imports refactored to not be cyclic). Luckily, when loading by IRIs, it's easy to avoid double parsing by adding the root ontology used at load time to the map of ontologies being imported, which is currently used to skip ultiple imports in the imports closure. The root was not included in this mechanism, for some reason - it should have been. Removing double parsing removes both issues, however the problem remains for root ontologies loaded from file or stream. --- .../cs/owl/owlapi/OWLOntologyManagerImpl.java | 36 ++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/impl/src/main/java/uk/ac/manchester/cs/owl/owlapi/OWLOntologyManagerImpl.java b/impl/src/main/java/uk/ac/manchester/cs/owl/owlapi/OWLOntologyManagerImpl.java index 3c2615ebbf..ced2fcb8fb 100644 --- a/impl/src/main/java/uk/ac/manchester/cs/owl/owlapi/OWLOntologyManagerImpl.java +++ b/impl/src/main/java/uk/ac/manchester/cs/owl/owlapi/OWLOntologyManagerImpl.java @@ -20,6 +20,7 @@ import static org.semanticweb.owlapi.util.OWLAPIPreconditions.optional; import static org.semanticweb.owlapi.util.OWLAPIPreconditions.verifyNotNull; import static org.semanticweb.owlapi.util.OWLAPIStreamUtils.asList; +import static org.semanticweb.owlapi.util.OWLAPIStreamUtils.asUnorderedSet; import java.io.File; import java.io.IOException; @@ -82,6 +83,7 @@ import org.semanticweb.owlapi.model.OWLDocumentFormat; import org.semanticweb.owlapi.model.OWLEntity; import org.semanticweb.owlapi.model.OWLImportsDeclaration; +import org.semanticweb.owlapi.model.OWLLogicalAxiom; import org.semanticweb.owlapi.model.OWLMutableOntology; import org.semanticweb.owlapi.model.OWLOntology; import org.semanticweb.owlapi.model.OWLOntologyAlreadyExistsException; @@ -730,9 +732,15 @@ private void checkForOntologyIDChange(OWLOntologyChange change) { OWLOntology o = setID.getOntology(); if (existingOntology != null && !o.equals(existingOntology) && !o.equalAxioms(existingOntology)) { - LOGGER.error("OWLOntologyManagerImpl.checkForOntologyIDChange() existing:{}", - existingOntology); - LOGGER.error("OWLOntologyManagerImpl.checkForOntologyIDChange() new:{}", o); + String location = "OWLOntologyManagerImpl.checkForOntologyIDChange()"; + LOGGER.error(location + " existing:{}", existingOntology); + LOGGER.error(location + " new:{}", o); + Set diff1 = asUnorderedSet(o.logicalAxioms()); + Set diff2 = asUnorderedSet(existingOntology.logicalAxioms()); + existingOntology.logicalAxioms().forEach(diff1::remove); + o.logicalAxioms().forEach(diff2::remove); + LOGGER.error(location + " only in existing:{}", diff2); + LOGGER.error(location + " only in new:{}", diff1); throw new OWLOntologyRenameException(setID.getChangeData(), setID.getNewOntologyID()); } renameOntology(setID.getOriginalOntologyID(), setID.getNewOntologyID()); @@ -895,7 +903,16 @@ public OWLOntology copyOntology(OWLOntology toCopy, OntologyCopy settings) @Override public OWLOntology loadOntology(IRI ontologyIRI) throws OWLOntologyCreationException { - return loadOntology(ontologyIRI, false, getOntologyLoaderConfiguration()); + // if an ontology cyclically imports itself, the manager should not try to download from the + // same URL twice. + Object value = new Object(); + if (!importedIRIs.containsKey(ontologyIRI)) { + importedIRIs.put(ontologyIRI, value); + } + OWLOntology loadOntology = + loadOntology(ontologyIRI, false, getOntologyLoaderConfiguration()); + importedIRIs.remove(ontologyIRI, value); + return loadOntology; } protected OWLOntology loadOntology(IRI iri, boolean allowExists, @@ -1068,6 +1085,17 @@ protected OWLOntology loadOntology(@Nullable IRI ontologyIRI, @Nullable protected OWLOntology load(OWLOntologyDocumentSource documentSource, OWLOntologyLoaderConfiguration configuration) throws OWLOntologyCreationException { + // Check if this is an IRI source and the IRI has already been loaded - this will stop + // ontologies + // that cyclically import themselves from being loaded twice. + if (documentSource instanceof IRIDocumentSource) { + IRIDocumentSource source = (IRIDocumentSource) documentSource; + Optional> findAny = documentIRIsByID.entrySet().stream() + .filter(v -> Objects.equals(source.getDocumentIRI(), v.getValue())).findAny(); + if (findAny.isPresent()) { + return getOntology(findAny.get().getKey()); + } + } for (OWLOntologyFactory factory : ontologyFactories) { if (factory.canAttemptLoading(documentSource)) { try {