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.

impl/src/main/java/uk/ac/manchester/cs/owl/owlapi/OWLOntologyManagerImpl.java

@@ -30,6 +30,8 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Objects;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
@@ -80,6 +82,7 @@
 import org.semanticweb.owlapi.model.OWLDocumentFormatImpl;
 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;
@@ -865,12 +868,18 @@ private void checkForOntologyIDChange(OWLOntologyChange change) {
             SetOntologyID setID = (SetOntologyID) change;
             OWLOntology existingOntology =
                 ontologiesByID.get(((SetOntologyID) change).getNewOntologyID());
-            if (existingOntology != null && !change.getOntology().equals(existingOntology)) {
-                if (!change.getOntology().getAxioms().equals(existingOntology.getAxioms())) {
-                    LOGGER.error("OWLOntologyManagerImpl.checkForOntologyIDChange() existing:{}",
-                        existingOntology);
-                    LOGGER.error("OWLOntologyManagerImpl.checkForOntologyIDChange() new:{}",
-                        change.getOntology());
+            OWLOntology o = change.getOntology();
+            if (existingOntology != null && !o.equals(existingOntology)) {
+                if (!o.getAxioms().equals(existingOntology.getAxioms())) {
+                    String location = "OWLOntologyManagerImpl.checkForOntologyIDChange()";
+                    LOGGER.error(location + " existing:{}", existingOntology);
+                    LOGGER.error(location + " new:{}", o);
+                    Set<OWLLogicalAxiom> diff1 = o.getLogicalAxioms();
+                    Set<OWLLogicalAxiom> diff2 = existingOntology.getLogicalAxioms();
+                    diff1.removeAll(existingOntology.getLogicalAxioms());
+                    diff2.removeAll(o.getLogicalAxioms());
+                    LOGGER.error(location + " only in existing:{}", diff2);
+                    LOGGER.error(location + " only in new:{}", diff1);
                     throw new OWLOntologyRenameException(change.getChangeData(),
                         ((SetOntologyID) change).getNewOntologyID());
                 }
@@ -1068,7 +1077,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;
     }
 
     @Nonnull
@@ -1252,6 +1270,19 @@ protected OWLOntology loadOntology(@Nullable IRI ontologyIRI,
 
     protected OWLOntology actualParse(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;
+            java.util.Optional<Entry<OWLOntologyID, IRI>> 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.canLoad(documentSource)) {
                 // Note - there is no need to add the ontology here,