Brett Porter wrote:
I tried to scan through this as quickly as I could. I haven't reviewed
the repository content classes at this point. I'm frankly not
comfortable with the size of this change and inclined to have it moved
to a branch until it can be more closely examined.
Here are my obligatory questions and comments...
On 10/10/2007, at 11:47 AM, [EMAIL PROTECTED] wrote:
- Have not swung all code over to new repositorycontent object yet.
what is left to do?
This was actually finished recently in revision 585588.
All of the code is now using the correct logic in RepositoryContent objects.
/**
* Purge the repo. Update db and index of removed artifacts.
*
* @param artifactFiles
* @throws RepositoryIndexException
*/
- protected void purge( File[] artifactFiles )
+ protected void purge( Set<ArtifactReference> references )
Was the behaviour of this changed? It looks like we now go File ->
ArtifactReference -> File which seems redundant?
Manipulating the repository directly via file references has caused many
bugs.
This is more of an interim step, while we work out the interface for
RepositoryContent to be more useful.
I expect things like this to go away completely once we do the
webservices work in post 1.0
For now, it's not harming anything, and just validates that what you are
attempting to purge is actual repository content.
This is similar to the discussion recently started around actions on the
repository being done via the RepositoryContent interface, not not via
direct filesystem manipulation. See that thread.
From what I can tell, you added functionality to delete support files
(great!), but it wasn't referenced in the commit log or a JIRA. It's
hard to track when multiple changes like that occur - can you clarify?
Something is not working, since as soon as I run it I get this:
INFO | jvm 1 | 2007/10/10 13:47:41 | 2007-10-10 13:47:41,036
[SocketListener0-4] ERROR
org.apache.maven.archiva.repository.scanner.RepositoryContentConsumers:default
- Consumer [repository-purge] had an error when processi
ng file
[/Applications/Archiva/archiva/data/repositories/snapshots/org/codehaus/plexus/plexus-expression-evaluator/1.0-alpha-2-SNAPSHOT/plexus-expression-evaluator-1.0-alpha-2-20070928.003615-2.jar]:
For input string: "alph"
INFO | jvm 1 | 2007/10/10 13:47:41 |
java.lang.NumberFormatException: For input string: "alph"
INFO | jvm 1 | 2007/10/10 13:47:41 | at
java.lang.NumberFormatException.forInputString(NumberFormatException.java:48)
INFO | jvm 1 | 2007/10/10 13:47:41 | at
java.lang.Integer.parseInt(Integer.java:447)
INFO | jvm 1 | 2007/10/10 13:47:41 | at
java.lang.Integer.parseInt(Integer.java:497)
+ System.err.println( "Purging: " +
artifactFile.getAbsolutePath() );
Oops.
+ System.err.println( "Requesting (retention) purge of " +
ArtifactReference.toKey( reference ) );
Oops.
Cleaned up in revision 584213.
Modified:
maven/archiva/trunk/archiva-base/archiva-consumers/archiva-core-consumers/src/test/conf/repository-manager-daysOld.xml
URL:
http://svn.apache.org/viewvc/maven/archiva/trunk/archiva-base/archiva-consumers/archiva-core-consumers/src/test/conf/repository-manager-daysOld.xml?rev=583412&r1=583411&r2=583412&view=diff
==============================================================================
---
maven/archiva/trunk/archiva-base/archiva-consumers/archiva-core-consumers/src/test/conf/repository-manager-daysOld.xml
(original)
+++
maven/archiva/trunk/archiva-base/archiva-consumers/archiva-core-consumers/src/test/conf/repository-manager-daysOld.xml
Wed Oct 10 02:47:20 2007
@@ -20,58 +20,47 @@
and....
Modified:
maven/archiva/trunk/archiva-base/archiva-consumers/archiva-core-consumers/src/test/conf/repository-manager.xml
URL:
http://svn.apache.org/viewvc/maven/archiva/trunk/archiva-base/archiva-consumers/archiva-core-consumers/src/test/conf/repository-manager.xml?rev=583412&r1=583411&r2=583412&view=diff
==============================================================================
---
maven/archiva/trunk/archiva-base/archiva-consumers/archiva-core-consumers/src/test/conf/repository-manager.xml
(original)
+++
maven/archiva/trunk/archiva-base/archiva-consumers/archiva-core-consumers/src/test/conf/repository-manager.xml
Wed Oct 10 02:47:20 2007
@@ -20,58 +20,47 @@
<configuration>
<version>1</version>
- <repositories>
- <repository>
+ <managedRepositories>
+ <managedRepository>
<id>internal</id>
<name>Archiva Managed Internal Repository</name>
- <url>file://${appserver.base}/repositories/internal</url>
+ <location>${appserver.base}/repositories/internal</location>
These changes are incorrect, if it is version '1', it is the old
format, you can't update the content (I'm surprised it works).
As far as I know, version 1 equals <managedRepositories> +
<remoteRepositories> etc...
This was required to allow the unit tests to operate.
Versions before 1 are in the realm of Archiva 0.9
And I don't see support for a version 2 in the code.
On 10/10/2007, at 11:47 AM, [EMAIL PROTECTED] wrote:
- assertFalse( el.getValue().equals( "20070315032817" ) );
+ // FIXME: assertFalse( el.getValue().equals(
"20070315032817" ) );
what happened here? (2 more instances in the file)
That was a holdover on MRM-535, it was known to be bad.
I should have labelled that as // FIXME [MRM-535] instead.
// check if metadata file was updated
- File artifactMetadataFile =
- new File(
"target/test/test-repo/org/apache/maven/plugins/maven-source-plugin/maven-metadata-local.xml"
);
+ File artifactMetadataFile = new File(
+
"target/test/test-repo/org/apache/maven/plugins/maven-source-plugin/maven-metadata.xml"
);
Did the file format change?
The assumption on maven-metadata-local.xml was bad.
Nothing in archiva uses it, consumes it, requests it, or works with it.
- try
+ if ( !artifact.getArtifactId().equalsIgnoreCase(
model.getArtifactId() ) )
why are artifact IDs case insensitive?
+ if ( !artifact.getVersion().equalsIgnoreCase(
model.getVersion() ) &&
+ !VersionUtil.getBaseVersion( artifact.getVersion()
).equalsIgnoreCase( model.getVersion() ) )
as above
This is to work around win32 issues with manual deploys, etc...
On 10/10/2007, at 11:47 AM, [EMAIL PROTECTED] wrote:
+ /* FIXME
public void
testLegacyRequestPluginConvertedToDefaultPathInManagedRepo()
throws Exception
{
@@ -400,7 +401,7 @@
setupTestableManagedRepository( path );
File expectedFile = new File( managedDefaultDir, path );
- ArtifactReference artifact = createArtifactReference(
"legacy", legacyPath );
+ ArtifactReference artifact =
managedDefaultRepository.toArtifactReference( path );
expectedFile.delete();
assertFalse( expectedFile.exists() );
@@ -415,6 +416,7 @@
assertFileEquals( expectedFile, downloadedFile, proxiedFile );
assertNoTempFiles( expectedFile );
}
+ */
what happened here? (2 instances)
This whole test case was invalidated with the RepositoryContent changes
and has been removed.
+ /**
+ * <p>
+ * Gather up the list of related artifacts to the
ArtifactReference provided.
+ * This typically inclues the pom files, and those things with
+ * classifiers (such as doc, source code, test libs, etc...)
+ * </p>
+ *
+ * <p>
+ * <strong>NOTE:</strong> Some layouts (such as maven 1
"legacy") are not compatible with this query.
+ * </p>
+ *
+ * @param reference the reference to work off of.
+ * @return the set of ArtifactReferences for related artifacts.
+ * @throws ContentNotFoundException if the initial artifact
reference does not exist within the repository.
+ * @throws LayoutException
+ */
+ public Set<ArtifactReference> getRelatedArtifacts(
ArtifactReference reference )
+ throws ContentNotFoundException, LayoutException;
How does this work if proxying? Or is it just for things already in
the repository (ie, for the purge).
Related artifacts are the things attached to the artifact.
Things like sources, javadoc, pom, etc...
There were a few places that had similar routines scattered around the
code, so it felt like a needed method.
But after a week or two of cleanup, even those methods no longer need
this concept.
This can be removed safely now.
On 10/10/2007, at 11:47 AM, [EMAIL PROTECTED] wrote:
+ private static final SimpleDateFormat lastUpdatedFormat;
+
+ static
+ {
+ lastUpdatedFormat = new SimpleDateFormat( "yyyyMMddHHmmss" );
+ lastUpdatedFormat.setTimeZone( DateUtils.UTC_TIME_ZONE );
+ }
This makes the metadata tools not thread safe and needs to be changed.
(I think I saw the same the purge changes).
Oh-kay.
I don't believe this will cause a thread safety issue.
But I can wrap a few synchronized blocks around access to those if it'll
make you feel better.
On 10/10/2007, at 11:47 AM, [EMAIL PROTECTED] wrote:
- /* TODO: Re-enabled in the future.
+ /*
Is this no longer going to be re-enabled? Are the tests valid? (3
instances)
+ /* TODO: Re-enabled in the future.
public void testGoodFooEjbClient()
Did this break?
+ /*
public void testGoodFooLibJavadoc()
What happened here? (2 instances)
Results of the Bad layout routines.
btw, those tests have been disabled for a *long* time.
Those tests have been migrated to the new RepositoryContent test cases
and work fine now.
The old tests have been removed from source control.
On 10/10/2007, at 11:47 AM, [EMAIL PROTECTED] wrote:
Added:
maven/archiva/trunk/archiva-base/archiva-repository-layer/src/test/resources/m1-repo-filelist.txt
URL:
http://svn.apache.org/viewvc/maven/archiva/trunk/archiva-base/archiva-repository-layer/src/test/resources/m1-repo-filelist.txt?rev=583412&view=auto
==============================================================================
---
maven/archiva/trunk/archiva-base/archiva-repository-layer/src/test/resources/m1-repo-filelist.txt
(added)
+++
maven/archiva/trunk/archiva-base/archiva-repository-layer/src/test/resources/m1-repo-filelist.txt
Wed Oct 10 02:47:20 2007
@@ -0,0 +1,6230 @@
+# Directory listing from people.apache.org
+# of path /www/people.apache.org/repo/m1-ibiblio-rsync-repository
+# Taken September 9, 2007
+#
+# File listing has been filtered using the following sed command.
+#
+# sed -e "/\.md5$/d" -e "/\.sha1$/d" -e "/\.asc$/d" -e "/\.meta$/d" \
+# -e "/LICENSE/d" -e "/\/licenses\//d"
+#
+# Any entries in here that are blatently wrong should be deleted.
+#
This is interesting as a one off, but not something I think we should
be running in the unit tests. It's way too huge.
I think this is important to test the effectiveness of the layout routines.
And it doesn't take that long.
Course, you can't see that as I apparently haven't checked in that unit
test. ;-)
Lemme see if I can find that on my laptop backup cd and get that commit'd.
--
- Joakim Erdfelt
[EMAIL PROTECTED]
Open Source Software (OSS) Developer
