Skip to content

Commit

Permalink
Further simplification of the ZIP publication implementation
Browse files Browse the repository at this point in the history
This is WIP, and I am kindly asking for a feedback.

This is a follow-up to PR #4156 and brings in a further simplification of the ZIP publication implementation.

In the mentioned PR we specifically implemented the functionality to use `project.group` value for the `artifactId` and if there was an override provided by user (ie the `groupId` value was explicitly specified in POM configuration) then it was used instead.

Further, we were explicitly setting the artifactId and version as well.

But in fact all this has been already happening under the hood (by the libraries that do the heavy lifting of publication tasks processing). There is really no need to (re-)implement it again. As we can see from the modified code and tests that we can perfectly remove almost all the ZIP publication initialization code and all the things still work as expected. And I think it is because this is how the Project Object Model (POM) was designed to work.

Because of that the condition to test the groupId presence: `if groupId == null` was useless. It was never `true`. If the `project.group` is not defined the publication task fails with an exception (we test it), if there is a custom `groupId` value setup in publication config then it overrides the `project.group` as well. Again, we test it. :-)

On top of that I am not really sure if we needed the condition to test: `if (mavenZip == null)` and create a new instance of MavenPublication if it is null. Hence I removed it and all the tests are still fine. Actually, I can not think of a case where the condition would be met (please prove me wrong).

Finally, not only this simplifies existing code but has some interesting consequences for the planned back-porting to `2.x` branch (which is expected to default to a hardcoded groupId `org.opensearch.plugin` unless user provides explicit override in POM configuration). But on that later. Let's now focus on these changes.

Signed-off-by: Lukáš Vlček <lukas.vlcek@aiven.io>
  • Loading branch information
lukas-vlcek committed Aug 31, 2022
1 parent d72861f commit e68289e
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 24 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Dependency updates (httpcore, mockito, slf4j, httpasyncclient, commons-codec) ([#4308](https://github.com/opensearch-project/OpenSearch/pull/4308))
- Use RemoteSegmentStoreDirectory instead of RemoteDirectory ([#4240](https://github.com/opensearch-project/OpenSearch/pull/4240))
- Plugin ZIP publication groupId value is configurable ([#4156](https://github.com/opensearch-project/OpenSearch/pull/4156))
- Further simplification of the ZIP publication implementation ([#4360](https://github.com/opensearch-project/OpenSearch/pull/4360))

### Deprecated

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,24 +42,7 @@ public static void configMaven(Project project) {
});
publishing.publications(publications -> {
MavenPublication mavenZip = (MavenPublication) publications.findByName(PUBLICATION_NAME);

if (mavenZip == null) {
mavenZip = publications.create(PUBLICATION_NAME, MavenPublication.class);
}

String groupId = mavenZip.getGroupId();
if (groupId == null) {
// The groupId is not customized thus we get the value from "project.group".
// See https://docs.gradle.org/current/userguide/publishing_maven.html#sec:identity_values_in_the_generated_pom
groupId = getProperty("group", project);
}

String artifactId = project.getName();
String pluginVersion = getProperty("version", project);
mavenZip.artifact(project.getTasks().named("bundlePlugin"));
mavenZip.setGroupId(groupId);
mavenZip.setArtifactId(artifactId);
mavenZip.setVersion(pluginVersion);
});
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ public void missingGroupValue() throws IOException, URISyntaxException, XmlPullP
}

/**
* This would be the most common use case where user declares Maven publication entity with basic info
* and the resulting POM file will use groupId and version values from the Gradle project object.
* This would be the most common use case where user declares Maven publication entity with minimal info
* and the resulting POM file will use artifactId, groupId and version values based on the Gradle project object.
*/
@Test
public void groupAndVersionValue() throws IOException, URISyntaxException, XmlPullParserException {
GradleRunner runner = prepareGradleRunnerFromTemplate("groupAndVersionValue.gradle");
public void useDefaultValues() throws IOException, URISyntaxException, XmlPullParserException {
GradleRunner runner = prepareGradleRunnerFromTemplate("useDefaultValues.gradle");
BuildResult result = runner.build();

/** Check if build and {@value ZIP_PUBLISH_TASK} tasks have run well */
Expand Down Expand Up @@ -108,7 +108,7 @@ public void groupAndVersionValue() throws IOException, URISyntaxException, XmlPu
).exists()
);

// Parse the maven file and validate the groupID
// Parse the maven file and validate default values
MavenXpp3Reader reader = new MavenXpp3Reader();
Model model = reader.read(
new FileReader(
Expand All @@ -130,6 +130,10 @@ public void groupAndVersionValue() throws IOException, URISyntaxException, XmlPu
);
assertEquals(model.getVersion(), "2.0.0.0");
assertEquals(model.getGroupId(), "org.custom.group");
assertEquals(model.getArtifactId(), PROJECT_NAME);
assertEquals(model.getName(), null);
assertEquals(model.getDescription(), null);

assertEquals(model.getUrl(), "https://github.com/doe/sample-plugin");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ publishing {
publications {
pluginZip(MavenPublication) {
pom {
name = "sample-plugin"
description = "pluginDescription"
// name = "plugin name"
// description = "plugin description"
licenses {
license {
name = "The Apache License, Version 2.0"
Expand Down

0 comments on commit e68289e

Please sign in to comment.