gnodet commented on code in PR #11576:
URL: https://github.com/apache/maven/pull/11576#discussion_r3284364996
##########
api/maven-api-xml/src/main/java/org/apache/maven/api/xml/XmlNode.java:
##########
@@ -55,47 +55,75 @@
public interface XmlNode {
/**
- * @deprecated since 4.0.0.
- * Use {@link XmlService#CHILDREN_COMBINATION_MODE_ATTRIBUTE}
instead.
+ * @deprecated Use {@link XmlService#CHILDREN_COMBINATION_MODE_ATTRIBUTE}
instead.
+ */
+ @Deprecated(since = "4.0.0", forRemoval = true)
+ String CHILDREN_COMBINATION_MODE_ATTRIBUTE =
XmlService.CHILDREN_COMBINATION_MODE_ATTRIBUTE;
Review Comment:
This introduces a **duplicate field declaration** of
`CHILDREN_COMBINATION_MODE_ATTRIBUTE` (the original declaration is still
present a few lines below at the old location). This will not compile.
It looks like the new Javadoc block for `CHILDREN_COMBINATION_MERGE` was
accidentally placed above the wrong field. The fix is to remove this duplicate
and move the `CHILDREN_COMBINATION_MERGE` Javadoc to above the existing
`CHILDREN_COMBINATION_MERGE` field:
```suggestion
* @deprecated Use {@link
XmlService#CHILDREN_COMBINATION_MODE_ATTRIBUTE} instead.
*/
@Deprecated(since = "4.0.0", forRemoval = true)
String CHILDREN_COMBINATION_MODE_ATTRIBUTE =
XmlService.CHILDREN_COMBINATION_MODE_ATTRIBUTE;
/**
* @deprecated Use {@link XmlService#CHILDREN_COMBINATION_MERGE} instead.
*/
@Deprecated(since = "4.0.0", forRemoval = true)
String CHILDREN_COMBINATION_MERGE =
XmlService.CHILDREN_COMBINATION_MERGE;
```
##########
api/maven-api-xml/src/main/java/org/apache/maven/api/xml/XmlNode.java:
##########
@@ -55,47 +55,75 @@
public interface XmlNode {
/**
- * @deprecated since 4.0.0.
- * Use {@link XmlService#CHILDREN_COMBINATION_MODE_ATTRIBUTE}
instead.
+ * @deprecated Use {@link XmlService#CHILDREN_COMBINATION_MODE_ATTRIBUTE}
instead.
+ */
+ @Deprecated(since = "4.0.0", forRemoval = true)
+ String CHILDREN_COMBINATION_MODE_ATTRIBUTE =
XmlService.CHILDREN_COMBINATION_MODE_ATTRIBUTE;
+
+ /**
+ * @deprecated Use {@link XmlService#CHILDREN_COMBINATION_MERGE} instead.
*/
@Deprecated(since = "4.0.0", forRemoval = true)
String CHILDREN_COMBINATION_MODE_ATTRIBUTE =
XmlService.CHILDREN_COMBINATION_MODE_ATTRIBUTE;
@Deprecated(since = "4.0.0", forRemoval = true)
String CHILDREN_COMBINATION_MERGE = XmlService.CHILDREN_COMBINATION_MERGE;
+
+ /**
+ * @deprecated Use {@link XmlService#CHILDREN_COMBINATION_APPEND} instead.
+ */
@Deprecated(since = "4.0.0", forRemoval = true)
String CHILDREN_COMBINATION_APPEND =
XmlService.CHILDREN_COMBINATION_APPEND;
+
/**
* This default mode for combining children DOMs during merge means that
where element names match, the process will
* try to merge the element data, rather than putting the dominant and
recessive elements (which share the same
* element name) as siblings in the resulting DOM.
+ *
+ * @deprecated Use {@link XmlService#DEFAULT_CHILDREN_COMBINATION_MODE}
instead.
*/
@Deprecated(since = "4.0.0", forRemoval = true)
String DEFAULT_CHILDREN_COMBINATION_MODE =
XmlService.DEFAULT_CHILDREN_COMBINATION_MODE;
+ /**
+ * @deprecated Use {@link XmlService#SELF_COMBINATION_MODE_ATTRIBUTE}
instead.
+ */
@Deprecated(since = "4.0.0", forRemoval = true)
String SELF_COMBINATION_MODE_ATTRIBUTE =
XmlService.SELF_COMBINATION_MODE_ATTRIBUTE;
-
+ /**
+ * @deprecated Use {@link XmlService#SELF_COMBINATION_OVERRIDE} instead.
+ */
@Deprecated(since = "4.0.0", forRemoval = true)
String SELF_COMBINATION_OVERRIDE = XmlService.SELF_COMBINATION_OVERRIDE;
+ /**
+ * @deprecated Use {@link XmlService#SELF_COMBINATION_MERGE} instead.
+ */
+
Review Comment:
Nit: stray blank line between the Javadoc closing `*/` and the `@Deprecated`
annotation. Should be removed for consistency with the other fields.
```suggestion
* @deprecated Use {@link XmlService#SELF_COMBINATION_MERGE} instead.
*/
@Deprecated(since = "4.0.0", forRemoval = true)
```
##########
impl/maven-core/src/test/java/org/apache/maven/internal/impl/DefaultProjectManagerTest.java:
##########
@@ -20,43 +20,102 @@
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.function.Supplier;
+import org.apache.maven.api.Language;
import org.apache.maven.api.ProducedArtifact;
import org.apache.maven.api.Project;
+import org.apache.maven.api.ProjectScope;
import org.apache.maven.api.services.ArtifactManager;
import org.apache.maven.impl.DefaultModelVersionParser;
+import org.apache.maven.impl.DefaultSourceRoot;
import org.apache.maven.impl.DefaultVersionParser;
import org.apache.maven.project.MavenProject;
import org.eclipse.aether.util.version.GenericVersionScheme;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.when;
class DefaultProjectManagerTest {
+ private DefaultProjectManager projectManager;
+
+ private Project project;
+
+ private ProducedArtifact artifact;
+
+ private Path artifactPath;
+
@Test
void attachArtifact() {
InternalMavenSession session =
Mockito.mock(InternalMavenSession.class);
ArtifactManager artifactManager = Mockito.mock(ArtifactManager.class);
MavenProject mavenProject = new MavenProject();
- Project project = new DefaultProject(session, mavenProject);
- ProducedArtifact artifact = Mockito.mock(ProducedArtifact.class);
- Path path = Paths.get("");
+ project = new DefaultProject(session, mavenProject);
+ artifact = Mockito.mock(ProducedArtifact.class);
+ artifactPath = Paths.get("");
DefaultVersionParser versionParser =
new DefaultVersionParser(new DefaultModelVersionParser(new
GenericVersionScheme()));
- DefaultProjectManager projectManager = new
DefaultProjectManager(session, artifactManager);
+ projectManager = new DefaultProjectManager(session, artifactManager);
mavenProject.setGroupId("myGroup");
mavenProject.setArtifactId("myArtifact");
mavenProject.setVersion("1.0-SNAPSHOT");
when(artifact.getGroupId()).thenReturn("myGroup");
when(artifact.getArtifactId()).thenReturn("myArtifact");
when(artifact.getBaseVersion()).thenReturn(versionParser.parseVersion("1.0-SNAPSHOT"));
- projectManager.attachArtifact(project, artifact, path);
+ projectManager.attachArtifact(project, artifact, artifactPath);
+ // Verify that an exception is thrown when the artifactId differs
when(artifact.getArtifactId()).thenReturn("anotherArtifact");
- assertThrows(IllegalArgumentException.class, () ->
projectManager.attachArtifact(project, artifact, path));
+ assertExceptionMessageContains("myGroup:myArtifact:1.0-SNAPSHOT",
"myGroup:anotherArtifact:1.0-SNAPSHOT");
+
+ // Add a Java module. It should relax the restriction on artifactId.
+ projectManager.addSourceRoot(
+ project,
+ new DefaultSourceRoot(
+ ProjectScope.MAIN,
+ Language.JAVA_FAMILY,
+ "org.foo.bar",
+ null,
+ Path.of("myProject"),
+ null,
+ null,
+ false,
+ null,
+ true));
+
+ // Verify that we get the same exception when the artifactId does not
match the module name
+ assertExceptionMessageContains("", "anotherArtifact");
+
+ // Verify that no exception is thrown when the artifactId is the
module name
+ when(artifact.getArtifactId()).thenReturn("org.foo.bar");
+ projectManager.attachArtifact(project, artifact, artifactPath);
+
+ // Verify that an exception is thrown when the groupId differs
+ when(artifact.getGroupId()).thenReturn("anotherGroup");
+ assertExceptionMessageContains("myGroup:myArtifact:1.0-SNAPSHOT",
"anotherGroup:org.foo.bar:1.0-SNAPSHOT");
+ }
+
+ /**
+ * Verifies that {@code projectManager.attachArtifact(…)} throws an
exception,
+ * and that the expecption message contains the expected and actual
<abbr>GAV</abbr>.
+ *
+ * @param expectedGAV the actual <abbr>GAV</abbr> that the exception
message should contain
Review Comment:
The `@param expectedGAV` description says "the actual GAV" -- it should say
"the expected GAV".
```suggestion
* @param expectedGAV the expected <abbr>GAV</abbr> that the exception
message should contain
```
##########
impl/maven-core/src/test/java/org/apache/maven/internal/impl/DefaultProjectManagerTest.java:
##########
@@ -20,43 +20,102 @@
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.function.Supplier;
+import org.apache.maven.api.Language;
import org.apache.maven.api.ProducedArtifact;
import org.apache.maven.api.Project;
+import org.apache.maven.api.ProjectScope;
import org.apache.maven.api.services.ArtifactManager;
import org.apache.maven.impl.DefaultModelVersionParser;
+import org.apache.maven.impl.DefaultSourceRoot;
import org.apache.maven.impl.DefaultVersionParser;
import org.apache.maven.project.MavenProject;
import org.eclipse.aether.util.version.GenericVersionScheme;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.when;
class DefaultProjectManagerTest {
+ private DefaultProjectManager projectManager;
+
+ private Project project;
+
+ private ProducedArtifact artifact;
+
+ private Path artifactPath;
+
@Test
void attachArtifact() {
InternalMavenSession session =
Mockito.mock(InternalMavenSession.class);
ArtifactManager artifactManager = Mockito.mock(ArtifactManager.class);
MavenProject mavenProject = new MavenProject();
- Project project = new DefaultProject(session, mavenProject);
- ProducedArtifact artifact = Mockito.mock(ProducedArtifact.class);
- Path path = Paths.get("");
+ project = new DefaultProject(session, mavenProject);
+ artifact = Mockito.mock(ProducedArtifact.class);
+ artifactPath = Paths.get("");
DefaultVersionParser versionParser =
new DefaultVersionParser(new DefaultModelVersionParser(new
GenericVersionScheme()));
- DefaultProjectManager projectManager = new
DefaultProjectManager(session, artifactManager);
+ projectManager = new DefaultProjectManager(session, artifactManager);
mavenProject.setGroupId("myGroup");
mavenProject.setArtifactId("myArtifact");
mavenProject.setVersion("1.0-SNAPSHOT");
when(artifact.getGroupId()).thenReturn("myGroup");
when(artifact.getArtifactId()).thenReturn("myArtifact");
when(artifact.getBaseVersion()).thenReturn(versionParser.parseVersion("1.0-SNAPSHOT"));
- projectManager.attachArtifact(project, artifact, path);
+ projectManager.attachArtifact(project, artifact, artifactPath);
+ // Verify that an exception is thrown when the artifactId differs
when(artifact.getArtifactId()).thenReturn("anotherArtifact");
- assertThrows(IllegalArgumentException.class, () ->
projectManager.attachArtifact(project, artifact, path));
+ assertExceptionMessageContains("myGroup:myArtifact:1.0-SNAPSHOT",
"myGroup:anotherArtifact:1.0-SNAPSHOT");
+
+ // Add a Java module. It should relax the restriction on artifactId.
+ projectManager.addSourceRoot(
+ project,
+ new DefaultSourceRoot(
+ ProjectScope.MAIN,
+ Language.JAVA_FAMILY,
+ "org.foo.bar",
+ null,
+ Path.of("myProject"),
+ null,
+ null,
+ false,
+ null,
+ true));
+
+ // Verify that we get the same exception when the artifactId does not
match the module name
+ assertExceptionMessageContains("", "anotherArtifact");
+
+ // Verify that no exception is thrown when the artifactId is the
module name
+ when(artifact.getArtifactId()).thenReturn("org.foo.bar");
+ projectManager.attachArtifact(project, artifact, artifactPath);
+
+ // Verify that an exception is thrown when the groupId differs
+ when(artifact.getGroupId()).thenReturn("anotherGroup");
+ assertExceptionMessageContains("myGroup:myArtifact:1.0-SNAPSHOT",
"anotherGroup:org.foo.bar:1.0-SNAPSHOT");
+ }
+
+ /**
+ * Verifies that {@code projectManager.attachArtifact(…)} throws an
exception,
+ * and that the expecption message contains the expected and actual
<abbr>GAV</abbr>.
Review Comment:
Typo: "expecption" -> "exception".
```suggestion
* and that the exception message contains the expected and actual
<abbr>GAV</abbr>.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
