Permalink
Browse files

Merge pull request #381 from sonatype/nexus-4970-ignore-corrupt-metadata

[NEXUS-4970] Skip merge of invalid metadata from group member repositories
  • Loading branch information...
2 parents 47fc3ef + dfc9682 commit 15816e7c8d7995e7cb29f3675e2f53c087b2ba4c @adreghiciu adreghiciu committed May 3, 2012
@@ -16,6 +16,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Iterator;
import java.util.List;
@@ -277,7 +278,21 @@ private StorageItem doRetrieveMetadata( ResourceStoreRequest request )
ops.add( new NexusMergeOperation( new MetadataOperand( existingMetadatas.get( i ) ) ) );
}
- MetadataBuilder.changeMetadata( result, ops );
+ final Collection<MetadataException> metadataExceptions = MetadataBuilder.changeMetadataIgnoringFailures(
+ result, ops
+ );
+ if ( metadataExceptions != null && !metadataExceptions.isEmpty() )
+ {
+ for ( final MetadataException metadataException : metadataExceptions )
+ {
+ getLogger().warn(
+ "Ignored exception during M2 metadata merging: "
+ + metadataException.getMessage()
+ + " (request " + request.getRequestPath() + ")",
+ metadataException
+ );
+ }
+ }
}
// build the result item
@@ -15,7 +15,9 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.List;
@@ -82,45 +84,87 @@ public static Metadata write( Metadata metadata, OutputStream out )
}
/**
- * apply a list of operators to the specified serialized Metadata object
+ * Apply a list of operators to the specified metadata.
*
- * @param metadataBytes - serialized Metadata object
- * @param mutators - operators
- * @return changed serialized object
+ * @param metadata - to be changed
+ * @param operations -
+ * @return changed metadata
* @throws MetadataException
*/
- public static void changeMetadata( Metadata metadata, List<MetadataOperation> mutators )
+ public static void changeMetadata( Metadata metadata, List<MetadataOperation> operations )
throws MetadataException
{
- boolean changed = false;
-
- if ( metadata == null )
- {
- metadata = new Metadata();
- }
-
// Uncomment these once the fixes are in place
// Version mdModelVersion = ModelVersionUtility.getModelVersion( metadata );
- if ( mutators != null && mutators.size() > 0 )
+ if ( metadata != null && operations != null && operations.size() > 0 )
{
- boolean currentChanged = false;
-
- for ( MetadataOperation op : mutators )
+ final Metadata clone = metadata.clone();
+ for ( MetadataOperation op : operations )
{
- currentChanged = op.perform( metadata );
+ op.perform( clone );
// if (currentChanged) {
// mdModelVersion = max of mdModelVersion and op.getModelVersion;
// }
-
- changed = currentChanged || changed;
}
+ replace( metadata, clone );
}
// ModelVersionUtility.setModelVersion( metadata, mdModelVersion );
}
+ /**
+ * Apply a list of operators to the specified metadata ignoring failing operations (failing operations will not
+ * affect original metadata).
+ *
+ * @param metadata - to be changed
+ * @param operations - operations to be applied to metadata
+ * @return collection of failing operations exceptions
+ */
+ public static Collection<MetadataException> changeMetadataIgnoringFailures(
+ final Metadata metadata,
+ final List<MetadataOperation> operations )
+ {
+ final Collection<MetadataException> failures = new ArrayList<MetadataException>();
+
+ if ( metadata != null && operations != null && operations.size() > 0 )
+ {
+ Metadata savePoint = metadata;
+ for ( MetadataOperation op : operations )
+ {
+ try
+ {
+ final Metadata clone = savePoint.clone();
+ op.perform( clone );
+ savePoint = clone;
+ }
+ catch ( MetadataException e )
+ {
+ failures.add( e );
+ }
+ }
+ replace( metadata, savePoint );
+ }
+
+ return failures;
+ }
+
+ private static void replace( final Metadata metadata, final Metadata newMetadata )
+ {
+ if ( metadata == null || newMetadata == null )
+ {
+ return;
+ }
+ metadata.setArtifactId( newMetadata.getArtifactId() );
+ metadata.setGroupId( newMetadata.getGroupId() );
+ metadata.setModelEncoding( newMetadata.getModelEncoding() );
+ metadata.setModelVersion( newMetadata.getModelVersion() );
+ metadata.setPlugins( newMetadata.getPlugins() );
+ metadata.setVersion( newMetadata.getVersion() );
+ metadata.setVersioning( newMetadata.getVersioning() );
+ }
+
public static void changeMetadata( Metadata metadata, MetadataOperation op )
throws MetadataException
{
@@ -0,0 +1,164 @@
+/**
+ * Sonatype Nexus (TM) Open Source Version
+ * Copyright (c) 2007-2012 Sonatype, Inc.
+ * All rights reserved. Includes the third-party code listed at http://links.sonatype.com/products/nexus/oss/attributions.
+ *
+ * This program and the accompanying materials are made available under the terms of the Eclipse Public License Version 1.0,
+ * which accompanies this distribution and is available at http://www.eclipse.org/legal/epl-v10.html.
+ *
+ * Sonatype Nexus (TM) Professional Version is available from Sonatype, Inc. "Sonatype" and "Sonatype Nexus" are trademarks
+ * of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
+ * Eclipse Foundation. All other trademarks are the property of their respective owners.
+ */
+package org.sonatype.nexus.proxy.maven;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+
+import java.util.Arrays;
+import java.util.Collection;
+
+import org.apache.maven.artifact.repository.metadata.Metadata;
+import org.apache.maven.artifact.repository.metadata.Versioning;
+import org.junit.Test;
+import org.sonatype.nexus.proxy.maven.metadata.operations.MetadataBuilder;
+import org.sonatype.nexus.proxy.maven.metadata.operations.MetadataException;
+import org.sonatype.nexus.proxy.maven.metadata.operations.MetadataOperation;
+import org.sonatype.sisu.litmus.testsupport.TestSupport;
+
+public class MetadataBuilderTest
+ extends TestSupport
+{
+
+ /**
+ * Tests that changing metadata will work on a clone of metadata and will actually not change the original metadata
+ * at all when one of the operations is failing.
+ * <p/>
+ * It also checks that operations to be performed after the failing operation will not be called.
+ */
+ @Test
+ public void metadataIsNotChangedWhenOperationFails()
+ {
+ final Metadata metadata = createMetadata();
+ final MetadataOperation notToBeCalledOperation = mock( MetadataOperation.class );
+
+ try
+ {
+ MetadataBuilder.changeMetadata(
+ metadata,
+ Arrays.asList(
+ new AddVersion30Operation(),
+ new FailAfterChangeOperation(),
+ notToBeCalledOperation
+ )
+ );
+ assertThat( "Changing metadata was supposed to fail", false, is( true ) );
+ }
+ catch ( MetadataException e )
+ {
+ // do nothing
+ }
+
+ assertThat( metadata.getVersion(), equalTo( "2.0" ) );
+ assertThat( metadata.getVersioning().getVersions().size(), is( 2 ) );
+ assertThat( metadata.getModelVersion(), equalTo( "1.0" ) );
+
+ verifyNoMoreInteractions( notToBeCalledOperation );
+ }
+
+ /**
+ * Tests that changing metadata will work on a clone of metadata and will actually change the metadata only with
+ * changes done by operations that does not fail.
+ * <p/>
+ * It also checks that changing will not fail but return a list of failures from failed operations.
+ *
+ * @throws MetadataException unexpected
+ */
+ @Test
+ public void failingOperationDoesNotChangeMetadata()
+ throws MetadataException
+ {
+ final Metadata metadata = createMetadata();
+
+ final Collection<MetadataException> metadataExceptions = MetadataBuilder.changeMetadataIgnoringFailures(
+ metadata,
+ Arrays.asList(
+ new AddVersion30Operation(),
+ new FailAfterChangeOperation(),
+ new SetModelEncodingOperation()
+ )
+ );
+
+ assertThat( metadata.getVersion(), equalTo( "3.0" ) );
+ assertThat( metadata.getVersioning().getVersions().size(), is( 3 ) );
+ assertThat( metadata.getModelVersion(), equalTo( "1.0" ) );
+ assertThat( metadata.getModelEncoding(), equalTo( "ISO-8859-1" ) );
+ assertThat( metadataExceptions, is( notNullValue() ) );
+ assertThat( metadataExceptions.size(), is( 1 ) );
+ }
+
+ private Metadata createMetadata()
+ {
+ final Metadata metadata = new Metadata();
+ metadata.setGroupId( "g" );
+ metadata.setArtifactId( "a" );
+ metadata.setModelEncoding( "UTF-8" );
+ metadata.setModelVersion( "1.0" );
+ metadata.setVersion( "2.0" );
+
+ final Versioning versioning = new Versioning();
+ versioning.addVersion( "1.0" );
+ versioning.addVersion( "2.0" );
+
+ metadata.setVersioning( versioning );
+
+ return metadata;
+ }
+
+ private static class AddVersion30Operation
+ implements MetadataOperation
+ {
+
+ @Override
+ public boolean perform( final Metadata metadata )
+ throws MetadataException
+ {
+ metadata.getVersioning().addVersion( "3.0" );
+ metadata.setVersion( "3.0" );
+
+ return true;
+ }
+ }
+
+ private static class FailAfterChangeOperation
+ implements MetadataOperation
+ {
+
+ @Override
+ public boolean perform( final Metadata metadata )
+ throws MetadataException
+ {
+ metadata.setModelVersion( "1.1" );
+ throw new MetadataException( "Wanted failure" );
+ }
+ }
+
+ private static class SetModelEncodingOperation
+ implements MetadataOperation
+ {
+
+ @Override
+ public boolean perform( final Metadata metadata )
+ throws MetadataException
+ {
+ metadata.setModelEncoding( "ISO-8859-1" );
+
+ return true;
+ }
+ }
+
+}
@@ -49,7 +49,7 @@ public Metadata doMergeAsM2GroupRepositoryDoes( final List<Metadata> existingMet
ops.add( new NexusMergeOperation( new MetadataOperand( existingMetadatas.get( i ) ) ) );
}
- MetadataBuilder.changeMetadata( result, ops );
+ MetadataBuilder.changeMetadataIgnoringFailures( result, ops );
return result;
}
@@ -235,6 +235,11 @@ public void testChecksum()
}
}
+ /**
+ * NEXUS-4970: merging should not fail by incompatible artifact ids ( incompatible one should be skipped).
+ *
+ * @throws Exception unexpected
+ */
@Test
public void testConflictMerge()
throws Exception
@@ -244,8 +249,6 @@ public void testConflictMerge()
try
{
getRootRouter().retrieveItem( new ResourceStoreRequest( "/groups/test" + mdPath, false ) );
-
- fail( "Should not be able to retrieve the maven-metadata.xml, since the merge should fail caused by incompatible artifactId." );
}
catch ( StorageException e )
{
@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<metadata>
+ <groupId>itext</groupId>
+ <artifactId>itext</artifactId>
+ <version>0.99</version>
+ <versioning>
+ <latest>1.3</latest>
+ <versions>
+ <version>0.99</version>
+ <version>1.02b</version>
+ <version>1.3.1</version>
+ <version>1.1.4</version>
+ <version>1.3</version>
+ </versions>
+ </versioning>
+</metadata>
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<metadata>
+ <groupId>rubygems</groupId>
+ <artifactId>itext</artifactId>
+ <versioning>
+ <versions>
+ <version>2.0.7</version>
+ </versions>
+ <lastUpdated>20110518041358</lastUpdated>
+ </versioning>
+</metadata>
Oops, something went wrong.

0 comments on commit 15816e7

Please sign in to comment.