PR: MRM-55

Submitted by: Maria Odea Ching

Checked the index for a duplicate and delete it if one exists

git-svn-id: https://svn.apache.org/repos/asf/maven/repository-manager/trunk@372138 13f79535-47bb-0310-9956-ffa450edef68

maven-repository-indexer/src/main/java/org/apache/maven/repository/indexing/AbstractRepositoryIndex.java

@@ -41,28 +41,42 @@ public abstract class AbstractRepositoryIndex
 
     protected ArtifactRepository repository;
 
+    protected boolean indexExists;
+
     /**
      * Class constructor
      *
      * @param indexPath
      * @param repository
-     * @param indexFields
      * @throws RepositoryIndexException
      */
-    protected AbstractRepositoryIndex( String indexPath, ArtifactRepository repository, String[] indexFields )
+    protected AbstractRepositoryIndex( String indexPath, ArtifactRepository repository )
         throws RepositoryIndexException
     {
         this.repository = repository;
         this.indexPath = indexPath;
+    }
 
+
+    public void open()
+        throws RepositoryIndexException
+    {
         try
         {
-            validateIndex( indexFields );
+            if ( indexExists )
-        }
-        catch ( IOException e )
             {
-            throw new RepositoryIndexException( e );
+                indexWriter = new IndexWriter( indexPath, getAnalyzer(), false );
             }
+            else
+            {
+                indexWriter = new IndexWriter( indexPath, getAnalyzer(), true );
+            }
+        }
+        catch ( IOException ie )
+        {
+            throw new RepositoryIndexException( ie );
+        }
+        indexOpen = true;
     }
 
     /**
@@ -146,11 +160,8 @@ public abstract class AbstractRepositoryIndex
      * @param indexFields
      * @throws RepositoryIndexException if the given indexPath is not valid for this type of RepositoryIndex
      */
-    private void validateIndex( String[] indexFields )
+    protected void validateIndex( String[] indexFields )
         throws RepositoryIndexException, IOException
-    {
-        File indexDir = new File( indexPath );
-        if ( IndexReader.indexExists( indexDir ) )
     {
         IndexReader indexReader = IndexReader.open( indexPath );
         try
@@ -173,21 +184,6 @@ public abstract class AbstractRepositoryIndex
             indexReader.close();
         }
     }
-        else if ( !indexDir.exists() )
-        {
-            indexWriter = new IndexWriter( indexPath, getAnalyzer(), true );
-        }
-        else if ( indexDir.isDirectory() )
-        {
-            throw new RepositoryIndexException( indexPath + " is not a valid index directory." );
-        }
-        else
-        {
-            throw new RepositoryIndexException( indexPath + " is not a directory." );
-        }
-
-        indexOpen = true;
-    }
 
     /**
      * @see org.apache.maven.repository.indexing.RepositoryIndex#getRepository()
@@ -198,9 +194,14 @@ public abstract class AbstractRepositoryIndex
     }
 
     /**
-     * @see org.apache.maven.repository.indexing.RepositoryIndex#deleteDocument(String, String)
+     * Delete the document(s) that contains the specified value on the specified field.
+     *
+     * @param field
+     * @param value
+     * @throws RepositoryIndexException
+     * @throws IOException
      */
-    public void deleteDocument( String field, String value )
+    protected void deleteDocument( String field, String value )
         throws RepositoryIndexException, IOException
     {
         IndexReader indexReader = null;
@@ -218,4 +219,44 @@ public abstract class AbstractRepositoryIndex
             indexReader.close();
         }
     }
+
+    /**
+     * Check if the index already exists.
+     *
+     * @throws IOException
+     * @throws RepositoryIndexException
+     */
+    protected void checkIfIndexExists()
+        throws IOException, RepositoryIndexException
+    {
+        File indexDir = new File( indexPath );
+
+        if ( IndexReader.indexExists( indexDir ) )
+        {
+            indexExists = true;
+        }
+        else if ( !indexDir.exists() )
+        {
+            indexExists = false;
+        }
+        else if ( indexDir.isDirectory() )
+        {
+            throw new RepositoryIndexException( indexPath + " is not a valid index directory." );
+        }
+        else
+        {
+            throw new RepositoryIndexException( indexPath + " is not a directory." );
+        }
+    }
+
+    /**
+     * Checks if the object has already been indexed.
+     *
+     * @param object the object to be indexed.
+     * @throws RepositoryIndexException
+     * @throws IOException
+     */
+    abstract void isIndexed( Object object )
+        throws RepositoryIndexException, IOException;
+
 }

maven-repository-indexer/src/main/java/org/apache/maven/repository/indexing/ArtifactRepositoryIndex.java

@@ -27,7 +27,9 @@ import org.apache.maven.repository.digest.Digester;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
 import java.util.Enumeration;
+import java.util.List;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipException;
 import java.util.zip.ZipFile;
@@ -70,6 +72,8 @@ public class ArtifactRepositoryIndex
 
     protected static final String ARTIFACT_TYPE = "ARTIFACT";
 
+    private static final List KEYWORD_FIELDS = Arrays.asList( new String[]{FLD_ID} );
+
     /**
      * Class constructor
      *
@@ -81,7 +85,7 @@ public class ArtifactRepositoryIndex
     public ArtifactRepositoryIndex( String indexPath, ArtifactRepository repository, Digester digester )
         throws RepositoryIndexException
     {
-        super( indexPath, repository, FIELDS );
+        super( indexPath, repository );
         this.digester = digester;
     }
 
@@ -98,6 +102,28 @@ public class ArtifactRepositoryIndex
         return analyzer;
     }
 
+    /**
+     * @see AbstractRepositoryIndex#isIndexed(Object)
+     */
+    public void isIndexed( Object object )
+        throws RepositoryIndexException, IOException
+    {
+        if ( object instanceof Artifact )
+        {
+            Artifact artifact = (Artifact) object;
+            checkIfIndexExists();
+            if ( indexExists )
+            {
+                validateIndex( FIELDS );
+                deleteDocument( FLD_ID, ARTIFACT_TYPE + artifact.getId() );
+            }
+        }
+        else
+        {
+            throw new RepositoryIndexException( "Object is not of type artifact." );
+        }
+    }
+
     /**
      * Method to index a given artifact
      *
@@ -107,11 +133,6 @@ public class ArtifactRepositoryIndex
     public void indexArtifact( Artifact artifact )
         throws RepositoryIndexException
     {
-        if ( !isOpen() )
-        {
-            throw new RepositoryIndexException( "Unable to add artifact index on a closed index" );
-        }
-
         StringBuffer classes = new StringBuffer();
         StringBuffer packages = new StringBuffer();
         StringBuffer files = new StringBuffer();
@@ -178,6 +199,11 @@ public class ArtifactRepositoryIndex
 
         try
         {
+            isIndexed( artifact );
+            if ( !isOpen() )
+            {
+                open();
+            }
             getIndexWriter().addDocument( doc );
         }
         catch ( IOException e )
@@ -191,7 +217,7 @@ public class ArtifactRepositoryIndex
      */
     public boolean isKeywordField( String field )
     {
-        return false;
+        return KEYWORD_FIELDS.contains( field );
     }
 
     /**

maven-repository-indexer/src/main/java/org/apache/maven/repository/indexing/MetadataRepositoryIndex.java

@@ -63,7 +63,7 @@ public class MetadataRepositoryIndex
     public MetadataRepositoryIndex( String indexPath, ArtifactRepository repository )
         throws RepositoryIndexException
     {
-        super( indexPath, repository, FIELDS );
+        super( indexPath, repository );
     }
 
     /**
@@ -115,11 +115,6 @@ public class MetadataRepositoryIndex
     private void indexMetadata( RepositoryMetadata repoMetadata )
         throws RepositoryIndexException
     {
-        if ( !isOpen() )
-        {
-            throw new RepositoryIndexException( "Unable to add artifact index on a closed index" );
-        }
-
         //get lastUpdated from Versioning (specified in Metadata object)
         //get pluginPrefixes from Plugin (spcified in Metadata object) -----> concatenate/append???
         //get the metadatapath: check where metadata is located, then concatenate the groupId,
@@ -177,6 +172,11 @@ public class MetadataRepositoryIndex
 
         try
         {
+            isIndexed( repoMetadata );
+            if ( !isOpen() )
+            {
+                open();
+            }
             getIndexWriter().addDocument( doc );
         }
         catch ( IOException e )
@@ -189,4 +189,26 @@ public class MetadataRepositoryIndex
     {
         return false;
     }
+
+    /**
+     * @see org.apache.maven.repository.indexing.AbstractRepositoryIndex#isIndexed(Object)
+     */
+    public void isIndexed( Object object )
+        throws RepositoryIndexException, IOException
+    {
+        if ( object instanceof RepositoryMetadata )
+        {
+            RepositoryMetadata repoMetadata = (RepositoryMetadata) object;
+            checkIfIndexExists();
+            if ( indexExists )
+            {
+                //validateIndex( FIELDS );
+                deleteDocument( FLD_ID, (String) repoMetadata.getKey() );
+            }
+        }
+        else
+        {
+            throw new RepositoryIndexException( "Object is not of type metadata." );
+        }
+    }
 }

maven-repository-indexer/src/main/java/org/apache/maven/repository/indexing/PomRepositoryIndex.java

@@ -98,7 +98,7 @@ public class PomRepositoryIndex
                                ArtifactFactory artifactFactory )
         throws RepositoryIndexException
     {
-        super( indexPath, repository, FIELDS );
+        super( indexPath, repository );
         this.digester = digester;
         this.artifactFactory = artifactFactory;
     }
@@ -116,6 +116,29 @@ public class PomRepositoryIndex
         return analyzer;
     }
 
+    /**
+     * @see org.apache.maven.repository.indexing.AbstractRepositoryIndex#isIndexed(Object)
+     */
+    public void isIndexed( Object object )
+        throws RepositoryIndexException, IOException
+    {
+        if ( object instanceof Model )
+        {
+            Model pom = (Model) object;
+            checkIfIndexExists();
+            if ( indexExists )
+            {
+                validateIndex( FIELDS );
+                deleteDocument( FLD_ID, POM_TYPE + pom.getId() );
+            }
+        }
+        else
+        {
+            throw new RepositoryIndexException( "Object is not of type model." );
+        }
+
+    }
+
     /**
      * Method to create the index fields for a Model object into the index
      *
@@ -125,11 +148,6 @@ public class PomRepositoryIndex
     public void indexPom( Model pom )
         throws RepositoryIndexException
     {
-        if ( !isOpen() )
-        {
-            throw new RepositoryIndexException( "Unable to add pom index on a closed index" );
-        }
-
         Document doc = new Document();
         doc.add( Field.Keyword( FLD_ID, POM_TYPE + pom.getId() ) );
         doc.add( Field.Text( FLD_GROUPID, pom.getGroupId() ) );
@@ -177,6 +195,11 @@ public class PomRepositoryIndex
 
         try
         {
+            isIndexed( pom );
+            if ( !isOpen() )
+            {
+                open();
+            }
             getIndexWriter().addDocument( doc );
         }
         catch ( IOException e )

maven-repository-indexer/src/test/java/org/apache/maven/repository/indexing/ArtifactRepositoryIndexingTest.java

@@ -69,11 +69,14 @@ public class ArtifactRepositoryIndexingTest
         throws Exception
     {
         RepositoryIndexingFactory factory = (RepositoryIndexingFactory) lookup( RepositoryIndexingFactory.ROLE );
+        Artifact artifact = getArtifact( "test", "test-artifactId", "1.0" );
+        artifact.setFile( new File( repository.getBasedir(), repository.pathOf( artifact ) ) );
 
         try
         {
             String notIndexDir = new File( "pom.xml" ).getAbsolutePath();
-            factory.createArtifactRepositoryIndex( notIndexDir, repository );
+            ArtifactRepositoryIndex indexer = factory.createArtifactRepositoryIndex( notIndexDir, repository );
+            indexer.indexArtifact( artifact );
             fail( "Must throw exception on non-directory index directory" );
         }
         catch ( RepositoryIndexException e )
@@ -84,7 +87,8 @@ public class ArtifactRepositoryIndexingTest
         try
         {
             String notIndexDir = new File( "" ).getAbsolutePath();
-            factory.createArtifactRepositoryIndex( notIndexDir, repository );
+            ArtifactRepositoryIndex indexer = factory.createArtifactRepositoryIndex( notIndexDir, repository );
+            indexer.indexArtifact( artifact );
             fail( "Must throw an exception on a non-index directory" );
         }
         catch ( RepositoryIndexException e )
@@ -92,43 +96,26 @@ public class ArtifactRepositoryIndexingTest
             // expected
         }
 
-        Artifact artifact = getArtifact( "test", "test-artifactId", "1.0" );
-        artifact.setFile( new File( repository.getBasedir(), repository.pathOf( artifact ) ) );
-
         ArtifactRepositoryIndex indexer = factory.createArtifactRepositoryIndex( indexPath, repository );
-        indexer.close();
-
         try
         {
-            indexer.indexArtifact( artifact );
+            indexer.isIndexed( new Object() );
-            fail( "Must throw exception on add index with closed index." );
+            fail( "Must throw exception on object not of type artifact." );
         }
         catch ( RepositoryIndexException e )
         {
             // expected
         }
-
-        try
-        {
-            indexer.optimize();
-            fail( "Must throw exception on optimize index with closed index." );
-        }
-        catch ( RepositoryIndexException e )
-        {
-            // expected
-        }
-
-        indexer = factory.createArtifactRepositoryIndex( indexPath, repository );
-
-        indexer.close();
     }
 
     /**
      * Create an index that will be used for testing.
+     * Indexing process: check if the object was already indexed [ checkIfIndexed(Object) ], open the index [ open() ],
+     * index the object [ index(Object) ], optimize the index [ optimize() ] and close the index [ close() ].
      *
      * @throws Exception
      */
-    public void createTestIndex()
+    private void createTestIndex()
         throws Exception
     {
         RepositoryIndexingFactory factory = (RepositoryIndexingFactory) lookup( RepositoryIndexingFactory.ROLE );
@@ -137,17 +124,27 @@ public class ArtifactRepositoryIndexingTest
         Artifact artifact = getArtifact( "org.apache.maven", "maven-artifact", "2.0.1" );
         artifact.setFile( new File( repository.getBasedir(), repository.pathOf( artifact ) ) );
         indexer.indexArtifact( artifact );
+        indexer.optimize();
+        indexer.close();
 
         artifact = getArtifact( "org.apache.maven", "maven-model", "2.0" );
         artifact.setFile( new File( repository.getBasedir(), repository.pathOf( artifact ) ) );
         indexer.indexArtifact( artifact );
+        indexer.optimize();
+        indexer.close();
 
         artifact = getArtifact( "test", "test-artifactId", "1.0" );
         artifact.setFile( new File( repository.getBasedir(), repository.pathOf( artifact ) ) );
         indexer.indexArtifact( artifact );
-
         indexer.optimize();
         indexer.close();
+
+        artifact = getArtifact( "test", "test-artifactId", "1.0" );
+        artifact.setFile( new File( repository.getBasedir(), repository.pathOf( artifact ) ) );
+        indexer.indexArtifact( artifact );
+        indexer.optimize();
+        indexer.close();
+
     }
 
     /**
@@ -275,7 +272,6 @@ public class ArtifactRepositoryIndexingTest
     public void testSearchCompound()
         throws Exception
     {
-
         createTestIndex();
 
         RepositoryIndexingFactory factory = (RepositoryIndexingFactory) lookup( RepositoryIndexingFactory.ROLE );
@@ -390,6 +386,11 @@ public class ArtifactRepositoryIndexingTest
         indexer.close();
     }
 
+    /**
+     * Test delete of document from the artifact index.
+     *
+     * @throws Exception
+     */
     public void testDeleteArtifactDocument()
         throws Exception
     {

maven-repository-indexer/src/test/java/org/apache/maven/repository/indexing/MetadataRepositoryIndexingTest.java

@@ -95,6 +95,8 @@ public class MetadataRepositoryIndexingTest
 
     /**
      * Create the test index.
+     * Indexing process: check if the object was already indexed [ checkIfIndexed(Object) ], open the index [ open() ],
+     * index the object [ index(Object) ], optimize the index [ optimize() ] and close the index [ close() ].
      *
      * @throws Exception
      */
@@ -107,15 +109,23 @@ public class MetadataRepositoryIndexingTest
         RepositoryMetadata repoMetadata =
             getMetadata( "org.apache.maven", null, null, "maven-metadata.xml", GROUP_TYPE );
         indexer.index( repoMetadata );
+        indexer.optimize();
+        indexer.close();
 
         repoMetadata =
             getMetadata( "org.apache.maven", "maven-artifact", "2.0.1", "maven-metadata.xml", ARTIFACT_TYPE );
         indexer.index( repoMetadata );
+        indexer.optimize();
+        indexer.close();
 
         repoMetadata =
             getMetadata( "org.apache.maven", "maven-artifact", "2.0.1", "maven-metadata.xml", SNAPSHOT_TYPE );
         indexer.index( repoMetadata );
+        indexer.optimize();
+        indexer.close();
 
+        repoMetadata = getMetadata( "org.apache.maven", null, null, "maven-metadata.xml", GROUP_TYPE );
+        indexer.index( repoMetadata );
         indexer.optimize();
         indexer.close();
     }
@@ -201,7 +211,7 @@ public class MetadataRepositoryIndexingTest
     public void testExceptions()
         throws Exception
     {
-        //test when the object passed in the index(..) method is not a RepositoryMetadat instance
+        //test when the object passed in the index(..) method is not a RepositoryMetadata instance
         RepositoryIndexingFactory factory = (RepositoryIndexingFactory) lookup( RepositoryIndexingFactory.ROLE );
         indexer = factory.createMetadataRepositoryIndex( indexPath, repository );
         try
@@ -209,37 +219,22 @@ public class MetadataRepositoryIndexingTest
             Artifact artifact = getArtifact( "org.apache.maven", "maven-artifact", "2.0.1" );
             indexer.index( artifact );
             fail( "Must throw exception when the passed object is not a RepositoryMetadata object." );
-        }
-        catch ( Exception e )
-        {
-        }
             indexer.optimize();
             indexer.close();
-
-        //test when the plugin prefix is blank
-        factory = (RepositoryIndexingFactory) lookup( RepositoryIndexingFactory.ROLE );
-        indexer = factory.createMetadataRepositoryIndex( indexPath, repository );
-        try
-        {
-            RepositoryMetadata repoMetadata = getMetadata( "test", null, null, "maven-metadata.xml", GROUP_TYPE );
-            indexer.index( repoMetadata );
         }
         catch ( Exception e )
         {
+            //expected
         }
-        indexer.optimize();
-        indexer.close();
 
-        //test when the index is closed
         try
         {
-            RepositoryMetadata repoMetadata =
+            indexer.isIndexed( new Object() );
-                getMetadata( "org.apache.maven", null, null, "maven-metadata.xml", GROUP_TYPE );
+            fail( "Must throw exception when the passed object is not of type metadata." );
-            indexer.index( repoMetadata );
-            fail( "Must throw exception when a metadata is added to the index while the indexer is still closed." );
         }
         catch ( Exception e )
         {
+            //expected
         }
     }
 

maven-repository-indexer/src/test/java/org/apache/maven/repository/indexing/PomRepositoryIndexingTest.java

@@ -74,11 +74,13 @@ public class PomRepositoryIndexingTest
         throws Exception
     {
         RepositoryIndexingFactory factory = (RepositoryIndexingFactory) lookup( RepositoryIndexingFactory.ROLE );
+        Model pom = getPom( "test", "test-artifactId", "1.0" );
 
         try
         {
             String notIndexDir = new File( "pom.xml" ).getAbsolutePath();
-            factory.createPomRepositoryIndex( notIndexDir, repository );
+            PomRepositoryIndex indexer = factory.createPomRepositoryIndex( notIndexDir, repository );
+            indexer.indexPom( pom );
             fail( "Must throw exception on non-directory index directory" );
         }
         catch ( RepositoryIndexException e )
@@ -89,7 +91,8 @@ public class PomRepositoryIndexingTest
         try
         {
             String notIndexDir = new File( "" ).getAbsolutePath();
-            factory.createPomRepositoryIndex( notIndexDir, repository );
+            PomRepositoryIndex indexer = factory.createPomRepositoryIndex( notIndexDir, repository );
+            indexer.indexPom( pom );
             fail( "Must throw an exception on a non-index directory" );
         }
         catch ( RepositoryIndexException e )
@@ -97,34 +100,16 @@ public class PomRepositoryIndexingTest
             // expected
         }
 
-        Model pom = getPom( "test", "test-artifactId", "1.0" );
-
         PomRepositoryIndex indexer = factory.createPomRepositoryIndex( indexPath, repository );
-        indexer.close();
-
         try
         {
-            indexer.indexPom( pom );
+            indexer.isIndexed( new Object() );
-            fail( "Must throw exception on add index with closed index." );
+            fail( "Must throw exception when the passed object is not of type model." );
         }
-        catch ( RepositoryIndexException e )
+        catch ( Exception e )
         {
             //expected
         }
-
-        try
-        {
-            indexer.optimize();
-            fail( "Must throw exception on optimize index with closed index." );
-        }
-        catch ( RepositoryIndexException e )
-        {
-            // expected
-        }
-
-        indexer = factory.createPomRepositoryIndex( indexPath, repository );
-
-        indexer.close();
     }
 
     /**
@@ -428,6 +413,8 @@ public class PomRepositoryIndexingTest
 
     /**
      * Create an index that will be used for testing.
+     * Indexing process: check if the object was already indexed [ checkIfIndexed(Object) ], open the index [ open() ],
+     * index the object [ index(Object) ], optimize the index [ optimize() ] and close the index [ close() ].
      *
      * @throws Exception
      */
@@ -439,13 +426,21 @@ public class PomRepositoryIndexingTest
 
         Model pom = getPom( "org.apache.maven", "maven-artifact", "2.0.1" );
         indexer.indexPom( pom );
+        indexer.optimize();
+        indexer.close();
 
         pom = getPom( "org.apache.maven", "maven-model", "2.0" );
         indexer.indexPom( pom );
+        indexer.optimize();
+        indexer.close();
 
         pom = getPom( "test", "test-artifactId", "1.0" );
         indexer.indexPom( pom );
+        indexer.optimize();
+        indexer.close();
 
+        pom = getPom( "test", "test-artifactId", "1.0" );
+        indexer.indexPom( pom );
         indexer.optimize();
         indexer.close();
     }