Add more tests for ontologies and fix a few minor issues

Raise an exception of forceIndex is used on an ontology that does not
have a cache name.

Exclude tests involving ontology providers as those are slow and depend
on external data.

Use the temporary directory by default if ontology.cache.dir is not set.

Author: arteymix

pom.xml

@@ -393,6 +393,8 @@
 						<include>**/*Test.java</include>
 					</includes>
 					<excludes>
+						<!-- these tests should be run manually since they require external data -->
+						<exclude>**/providers/*Test.java</exclude>
 						<exclude>**/AllTests.java</exclude>
 						<exclude>**/gui/Test*.java</exclude>
 						<exclude>**/AbstractTest*.java</exclude>

src/ontology.properties

@@ -1,4 +1,4 @@
-
+# set to true to enable specific ontologies
 load.diseaseOntology=false
 load.fmaOntology=false
 load.chebiOntology=false
@@ -32,10 +32,9 @@ url.uberonOntology=http://purl.obolibrary.org/obo/uberon.owl
 url.efOntology=https://www.ebi.ac.uk/efo/efo.owl
 url.obiOntology=http://purl.obolibrary.org/obo/obi.owl
 
-
 # no longer actively used.
 url.fmaOntology=http://purl.obolibrary.org/obo/fma.owl
 
-ontology.index.dir=ontology.index.dir
+ontology.index.dir=
 ontology.cache.dir=
 ncbo.api.key=

src/ubic/basecode/ontology/jena/AbstractOntologyMemoryBackedService.java

@@ -16,7 +16,6 @@
 
 import com.hp.hpl.jena.ontology.OntModel;
 
-import javax.annotation.Nullable;
 import java.io.InputStream;
 
 /**
@@ -33,7 +32,7 @@ protected boolean getProcessImport() {
 
     @Override
     protected OntModel loadModel() {
-        return OntologyLoader.loadMemoryModel( this.getOntologyUrl(), this.getOntologyName(), this.getProcessImport() );
+        return OntologyLoader.loadMemoryModel( this.getOntologyUrl(), this.getCacheName(), this.getProcessImport() );
     }
 
     @Override

src/ubic/basecode/ontology/jena/AbstractOntologyService.java

@@ -44,6 +44,7 @@
 import ubic.basecode.util.Configuration;
 
 import javax.annotation.Nullable;
+import java.io.IOException;
 import java.io.InputStream;
 import java.util.*;
 import java.util.concurrent.locks.Lock;
@@ -108,6 +109,15 @@ private void initialize( @Nullable InputStream stream, boolean forceLoad, boolea
         String ontologyName = getOntologyName();
         String cacheName = getCacheName();
 
+        // Detect configuration problems.
+        if ( StringUtils.isBlank( ontologyUrl ) ) {
+            throw new IllegalStateException( "URL not defined for %s: ontology cannot be loaded. (" + this + ")" );
+        }
+
+        if ( cacheName == null && forceIndexing ) {
+            throw new IllegalArgumentException( String.format( "No cache directory is set for %s, cannot force indexing.", this ) );
+        }
+
         boolean loadOntology = isEnabled();
 
         // If loading ontologies is disabled in the configuration, return
@@ -117,16 +127,6 @@ private void initialize( @Nullable InputStream stream, boolean forceLoad, boolea
             return;
         }
 
-        // Detect configuration problems.
-        if ( StringUtils.isBlank( ontologyUrl ) ) {
-            throw new IllegalStateException( "URL not defined for %s: ontology cannot be loaded. (" + this + ")" );
-        }
-
-        // This thread indexes ontology and creates local cache for uri->ontology terms mappings.
-        if ( !forceIndexing ) {
-            log.info( "{} index will *not* be refreshed unless the ontology has changed or the index is missing", this );
-        }
-
         log.info( "Loading ontology: {}...", this );
         StopWatch loadTime = StopWatch.createStarted();
 
@@ -150,22 +150,18 @@ private void initialize( @Nullable InputStream stream, boolean forceLoad, boolea
                 .filterKeep( new RestrictionWithOnPropertyFilter( additionalProperties ) )
                 .toSet();
 
-        //Checks if the current ontology has changed since it was last loaded.
-        boolean changed = cacheName == null || OntologyLoader.hasChanged( cacheName );
-        boolean indexExists = cacheName != null && OntologyIndexer.getSubjectIndex( cacheName ) != null;
-        boolean forceReindexing = forceLoad && forceIndexing;
-
-        /*
-         * Indexing is slow, don't do it if we don't have to.
-         */
-        boolean force = forceReindexing || changed || !indexExists;
-
         // indexing is lengthy, don't bother if we're interrupted
         if ( checkIfInterrupted() )
             return;
 
         if ( cacheName != null ) {
-            index = OntologyIndexer.indexOntology( cacheName, model, force );
+            //Checks if the current ontology has changed since it was last loaded.
+            boolean changed = OntologyLoader.hasChanged( cacheName );
+            boolean indexExists = OntologyIndexer.getSubjectIndex( cacheName ) != null;
+            boolean forceReindexing = forceLoad && forceIndexing;
+            // indexing is slow, don't do it if we don't have to.
+            index = OntologyIndexer.indexOntology( cacheName, model,
+                    forceReindexing || changed || !indexExists );
         } else {
             index = null;
         }
@@ -183,7 +179,11 @@ private void initialize( @Nullable InputStream stream, boolean forceLoad, boolea
             this.isInitialized = true;
             if ( cacheName != null ) {
                 // now that the terms have been replaced, we can clear old caches
-                OntologyLoader.deleteOldCache( cacheName );
+                try {
+                    OntologyLoader.deleteOldCache( cacheName );
+                } catch ( IOException e ) {
+                    log.error( String.format( String.format( "Failed to delete old cache directory for %s.", this ), e ) );
+                }
             }
         } finally {
             lock.unlock();