add some design notes, and take some other notes about design deficiencies that need to be corrected

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

maven-repository-core/src/main/java/org/apache/maven/repository/scheduler/IndexerTask.java

@@ -111,6 +111,7 @@ public class IndexerTask
                 indexArtifact( artifacts, indexPath, defaultRepository );
             }
 
+            // TODO: I believe this is incorrect, since it only discovers standalone POMs, not the individual artifacts!
             List models = discoverer.discoverStandalonePoms( defaultRepository, blacklistedPatterns, includeSnapshots );
             if ( !models.isEmpty() )
             {

maven-repository-discovery/src/main/java/org/apache/maven/repository/discovery/ArtifactDiscoverer.java

@@ -26,6 +26,9 @@ import java.util.List;
  *
  * @author John Casey
  * @author Brett Porter
+ * @todo do we want blacklisted patterns in another form? Part of the object construction?
+ * @todo should includeSnapshots be configuration on the component? If not, should the methods be changed to include alternates for both possibilities (discoverReleaseArtifacts, discoverReleaseAndSnapshotArtifacts)?
+ * @todo instead of a returned list, should a listener be passed in?
  */
 public interface ArtifactDiscoverer
     extends Discoverer
@@ -39,9 +42,6 @@ public interface ArtifactDiscoverer
      * @param blacklistedPatterns pattern that lists any files to prevent from being included when scanning
      * @param includeSnapshots    whether to discover snapshots
      * @return the list of artifacts discovered
-     * @todo do we want blacklisted patterns in another form? Part of the object construction?
-     * @todo should includeSnapshots be configuration on the component?
-     * @todo instead of a returned list, should a listener be passed in?
      */
     List discoverArtifacts( ArtifactRepository repository, String blacklistedPatterns, boolean includeSnapshots );
 
@@ -52,9 +52,7 @@ public interface ArtifactDiscoverer
      * @param blacklistedPatterns pattern that lists any files to prevent from being included when scanning
      * @param includeSnapshots    whether to discover snapshots
      * @return the list of artifacts discovered
-     * @todo do we want blacklisted patterns in another form? Part of the object construction?
-     * @todo should includeSnapshots be configuration on the component?
-     * @todo instead of a returned list, should a listener be passed in?
+     * @todo why do we need this? shouldn't the discovered artifacts above link to the related POM, and include standalone POMs? Why would we need just this list?
      */
     List discoverStandalonePoms( ArtifactRepository repository, String blacklistedPatterns, boolean includeSnapshots );
 
@@ -63,6 +61,7 @@ public interface ArtifactDiscoverer
      *
      * @param path the path
      * @return the artifact
+     * @throws DiscovererException if the file is not a valid artifact
      * @todo this should be in maven-artifact
      */
     Artifact buildArtifact( String path )

maven-repository-discovery/src/main/java/org/apache/maven/repository/discovery/DefaultMetadataDiscoverer.java

@@ -23,8 +23,8 @@ import org.apache.maven.artifact.repository.metadata.Metadata;
 import org.apache.maven.artifact.repository.metadata.RepositoryMetadata;
 import org.apache.maven.artifact.repository.metadata.SnapshotArtifactRepositoryMetadata;
 import org.apache.maven.artifact.repository.metadata.io.xpp3.MetadataXpp3Reader;
-import org.codehaus.plexus.util.xml.pull.XmlPullParserException;
 import org.codehaus.plexus.util.StringUtils;
+import org.codehaus.plexus.util.xml.pull.XmlPullParserException;
 
 import java.io.File;
 import java.io.IOException;
@@ -50,17 +50,14 @@ public class DefaultMetadataDiscoverer
 {
     /**
      * Standard patterns to include in discovery of metadata files.
+     *
+     * @todo do we really need all these paths? Add tests for all 3 levels and confirm only 2 are needed.
      */
-    private static final String[] STANDARD_DISCOVERY_INCLUDES = {"**/*-metadata.xml",
-                                                                 "**/*/*-metadata.xml",
-                                                                 "**/*/*/*-metadata.xml",
-                                                                 "**/*-metadata-*.xml",
-                                                                 "**/*/*-metadata-*.xml",
-                                                                 "**/*/*/*-metadata-*.xml"
-                                                                };
+    private static final String[] STANDARD_DISCOVERY_INCLUDES = {"**/*-metadata.xml", "**/*/*-metadata.xml",
+        "**/*/*/*-metadata.xml", "**/*-metadata-*.xml", "**/*/*-metadata-*.xml", "**/*/*/*-metadata-*.xml"};
 
     /**
-     * @see org.apache.maven.repository.discovery.MetadataDiscoverer#discoverMetadata(java.io.File, String)
+     * @see org.apache.maven.repository.discovery.MetadataDiscoverer#discoverMetadata(java.io.File,String)
      */
     public List discoverMetadata( File repositoryBase, String blacklistedPatterns )
     {
@@ -112,8 +109,8 @@ public class DefaultMetadataDiscoverer
         catch ( MalformedURLException e )
         {
             // shouldn't happen
-            throw new DiscovererException( "Error constructing metadata file '" + repoPath + "': " +
-                                           e.getMessage(), e );
+            throw new DiscovererException( "Error constructing metadata file '" + repoPath + "': " + e.getMessage(),
+                                           e );
         }
         catch ( IOException e )
         {
@@ -133,7 +130,7 @@ public class DefaultMetadataDiscoverer
     /**
      * Builds a RepositoryMetadata object from a Metadata object and its path
      *
-     * @param m Metadata
+     * @param m            Metadata
      * @param metadataPath path
      * @return RepositoryMetadata if the parameters represent one; null if not
      */

maven-repository-discovery/src/site/apt/design.apt

@@ -0,0 +1,37 @@
+ -----
+ Discoverer Design
+ -----
+ Brett Porter
+ -----
+ 24 July 2006
+ -----
+
+Discoverer Design
+
+  The artifact discoverer is designed to traverse the paths in the repository and identify files that are part of
+  a legitimate artifact.
+
+  There are two plexus components available:
+
+    * {{{../apidocs/org/apache/maven/repository/discovery/ArtifactDiscoverer.html} ArtifactDiscoverer}}
+
+    * {{{../apidocs/org/apache/maven/repository/discovery/MetadataDiscoverer.html} MetadataDiscoverer}}
+
+  Each of these components currently have an implementation for the both <<<legacy>>> and <<<default>>> repository
+  layouts.
+
+  The artifact discoverer will find all artifacts in the repository, while metadata discovery finds any
+  <<<maven-metadata.xml>>> files (both remote and local repository formats).
+
+  * Limitations
+
+    * In the artifact discoverer, POMs will be identified as separate artifacts to their related artifacts, as will each
+      individual derivative artifact at present. Later, these will be linked - see
+      {{{http://jira.codehaus.org/browse/MRM-40} MRM-40}}.
+
+    * Currently, deleted artifacts are not tracked. This requires a separate event - see
+      {{{http://jira.codehaus.org/browse/MRM-37} MRM-37}}.
+
+    * Currently, all artifacts are discovered instead of just those changed since the last discovery for a particular
+      operation - see {{{http://jira.codehaus.org/browse/MRM-125} MRM-125}}.
+

maven-repository-discovery/src/site/site.xml

@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="ISO-8859-1"?>
+<!--
+  ~ Copyright 2005-2006 The Apache Software Foundation.
+  ~
+  ~ Licensed under the Apache License, Version 2.0 (the "License");
+  ~ you may not use this file except in compliance with the License.
+  ~ You may obtain a copy of the License at
+  ~
+  ~      http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing, software
+  ~ distributed under the License is distributed on an "AS IS" BASIS,
+  ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  ~ See the License for the specific language governing permissions and
+  ~ limitations under the License.
+  -->
+
+<project>
+  <body>
+    <menu name="Design Documentation">
+      <item name="Discoverer Design" href="/design.html"/>
+    </menu>
+  </body>
+</project>

maven-repository-discovery/src/test/java/org/apache/maven/repository/discovery/DefaultArtifactDiscovererTest.java

@@ -605,6 +605,42 @@ public class DefaultArtifactDiscovererTest
         assertEquals( createArtifact( "javax.sql", "jdbc", "2.0" ), artifact );
     }
 
+    public void testUpdatedInRepository()
+        throws ComponentLookupException
+    {
+        String testPath = "javax/sql/jdbc/2.0/jdbc-2.0.jar";
+
+        Artifact artifact = getArtifactFromPath( testPath );
+
+        assertNotNull( "Normal artifact path error", artifact );
+
+        assertEquals( createArtifact( "javax.sql", "jdbc", "2.0" ), artifact );
+    }
+
+    public void testNotUpdatedInRepository()
+        throws ComponentLookupException
+    {
+        String testPath = "javax/sql/jdbc/2.0/jdbc-2.0.jar";
+
+        Artifact artifact = getArtifactFromPath( testPath );
+
+        assertNotNull( "Normal artifact path error", artifact );
+
+        assertEquals( createArtifact( "javax.sql", "jdbc", "2.0" ), artifact );
+    }
+
+    public void testNotUpdatedInRepositoryForcedDiscovery()
+        throws ComponentLookupException
+    {
+        String testPath = "javax/sql/jdbc/2.0/jdbc-2.0.jar";
+
+        Artifact artifact = getArtifactFromPath( testPath );
+
+        assertNotNull( "Normal artifact path error", artifact );
+
+        assertEquals( createArtifact( "javax.sql", "jdbc", "2.0" ), artifact );
+    }
+
     public void testSnapshotWithClassifier()
         throws ComponentLookupException
     {

src/site/site.xml

@@ -21,7 +21,7 @@
       <item name="Maven" href="http://maven.apache.org/"/>
     </links>
 
-    <menu ref="reports"/>
+    <menu ref="reports" inherit="bottom"/>
 
   </body>
   <skin>