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 opensearch-project#4156 and brings in a further simplification of the ZIP publication implementation and a new gradle test.

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).

I added a new test to verify we can run "publishToMavenLocal" and get expected results. The inspiration for this comes from opensearch-project/opensearch-plugin-template-java#35

Signed-off-by: Lukáš Vlček <lukas.vlcek@aiven.io>
  • Loading branch information
lukas-vlcek committed Sep 12, 2022
1 parent 54364a5 commit 91a3fac
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 60 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Plugin ZIP publication groupId value is configurable ([#4156](https://github.com/opensearch-project/OpenSearch/pull/4156))
- Add index specific setting for remote repository ([#4253](https://github.com/opensearch-project/OpenSearch/pull/4253))
- [Segment Replication] Update replicas to commit SegmentInfos instead of relying on SIS files from primary shards. ([#4402](https://github.com/opensearch-project/OpenSearch/pull/4402))
- Further simplification of the ZIP publication implementation ([#4360](https://github.com/opensearch-project/OpenSearch/pull/4360))

### Deprecated

Expand Down
81 changes: 35 additions & 46 deletions buildSrc/src/main/java/org/opensearch/gradle/pluginzip/Publish.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@

import org.gradle.api.Plugin;
import org.gradle.api.Project;
import org.gradle.api.logging.Logger;
import org.gradle.api.logging.Logging;
import org.gradle.api.publish.PublishingExtension;
import org.gradle.api.publish.maven.MavenPublication;
import org.gradle.api.publish.maven.plugins.MavenPublishPlugin;
Expand All @@ -20,72 +18,63 @@

public class Publish implements Plugin<Project> {

private static final Logger LOGGER = Logging.getLogger(Publish.class);

public final static String EXTENSION_NAME = "zipmavensettings";
// public final static String EXTENSION_NAME = "zipmavensettings";
// public final static String PLUGIN_ZIP_PUBLISH_POM_TASK = "generatePomFileForPluginZipPublication";
// public final static String LOCALMAVEN = "publishToMavenLocal";
// TODO: we can remove these ^^
public final static String PUBLICATION_NAME = "pluginZip";
public final static String STAGING_REPO = "zipStaging";
public final static String PLUGIN_ZIP_PUBLISH_POM_TASK = "generatePomFileForPluginZipPublication";
public final static String LOCALMAVEN = "publishToMavenLocal";
public final static String LOCAL_STAGING_REPO_PATH = "/build/local-staging-repo";
public String zipDistributionLocation = "/build/distributions/";
// TODO: the path ^^ needs to use platform dependant file separators

public static void configMaven(Project project) {
private boolean isZipPublicationPresent(Project project) {
PublishingExtension pe = project.getExtensions().findByType(PublishingExtension.class);
if (pe == null) {
return false;
}
return pe.getPublications().findByName(PUBLICATION_NAME) != null;
}

private void addLocalMavenRepo(Project project) {
final Path buildDirectory = project.getRootDir().toPath();
project.getPluginManager().apply(MavenPublishPlugin.class);
// project.getPluginManager().apply(MavenPublishPlugin.class);
// TODO: why is this ^^ needed?
project.getExtensions().configure(PublishingExtension.class, publishing -> {
publishing.repositories(repositories -> {
repositories.maven(maven -> {
maven.setName(STAGING_REPO);
maven.setUrl(buildDirectory.toString() + LOCAL_STAGING_REPO_PATH);
});
});
});
}

private void addZipArtifact(Project project) {
project.getExtensions().configure(PublishingExtension.class, publishing -> {
publishing.publications(publications -> {
MavenPublication mavenZip = (MavenPublication) publications.findByName(PUBLICATION_NAME);

if (mavenZip == null) {
mavenZip = publications.create(PUBLICATION_NAME, MavenPublication.class);
if (mavenZip != null ) {
mavenZip.artifact(project.getTasks().named("bundlePlugin"));
}

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);
});
});
}

static String getProperty(String name, Project project) {
if (project.hasProperty(name)) {
Object property = project.property(name);
if (property != null) {
return property.toString();
}
}
return null;
}

@Override
public void apply(Project project) {
project.afterEvaluate(evaluatedProject -> {
configMaven(project);
Task validatePluginZipPom = project.getTasks().findByName("validatePluginZipPom");
if (validatePluginZipPom != null) {
project.getTasks().getByName("validatePluginZipPom").dependsOn("generatePomFileForNebulaPublication");
}
Task publishPluginZipPublicationToZipStagingRepository = project.getTasks()
.findByName("publishPluginZipPublicationToZipStagingRepository");
if (publishPluginZipPublicationToZipStagingRepository != null) {
publishPluginZipPublicationToZipStagingRepository.dependsOn("generatePomFileForNebulaPublication");
if (isZipPublicationPresent(project)) {
addLocalMavenRepo(project);
addZipArtifact(project);
Task validatePluginZipPom = project.getTasks().findByName("validatePluginZipPom");
if (validatePluginZipPom != null) {
validatePluginZipPom.dependsOn("generatePomFileForNebulaPublication");
}
Task publishPluginZipPublicationToZipStagingRepository = project.getTasks()
.findByName("publishPluginZipPublicationToZipStagingRepository");
if (publishPluginZipPublicationToZipStagingRepository != null) {
publishPluginZipPublicationToZipStagingRepository.dependsOn("generatePomFileForNebulaPublication");
}
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,18 @@ public void tearDown() {

@Test
public void missingGroupValue() throws IOException, URISyntaxException, XmlPullParserException {
GradleRunner runner = prepareGradleRunnerFromTemplate("missingGroupValue.gradle");
GradleRunner runner = prepareGradleRunnerFromTemplate("missingGroupValue.gradle", "build", ZIP_PUBLISH_TASK);
Exception e = assertThrows(UnexpectedBuildFailure.class, runner::build);
assertTrue(e.getMessage().contains("Invalid publication 'pluginZip': groupId cannot be empty."));
}

/**
* 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", "build", ZIP_PUBLISH_TASK);
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 All @@ -139,7 +143,7 @@ public void groupAndVersionValue() throws IOException, URISyntaxException, XmlPu
*/
@Test
public void missingPOMEntity() throws IOException, URISyntaxException, XmlPullParserException {
GradleRunner runner = prepareGradleRunnerFromTemplate("missingPOMEntity.gradle");
GradleRunner runner = prepareGradleRunnerFromTemplate("missingPOMEntity.gradle", "build", ZIP_PUBLISH_TASK);
BuildResult result = runner.build();

/** Check if build and {@value ZIP_PUBLISH_TASK} tasks have run well */
Expand Down Expand Up @@ -186,7 +190,7 @@ public void missingPOMEntity() throws IOException, URISyntaxException, XmlPullPa
*/
@Test
public void customizedGroupValue() throws IOException, URISyntaxException, XmlPullParserException {
GradleRunner runner = prepareGradleRunnerFromTemplate("customizedGroupValue.gradle");
GradleRunner runner = prepareGradleRunnerFromTemplate("customizedGroupValue.gradle", "build", ZIP_PUBLISH_TASK);
BuildResult result = runner.build();

/** Check if build and {@value ZIP_PUBLISH_TASK} tasks have run well */
Expand Down Expand Up @@ -223,21 +227,97 @@ public void customizedGroupValue() throws IOException, URISyntaxException, XmlPu
*/
@Test
public void customizedInvalidGroupValue() throws IOException, URISyntaxException {
GradleRunner runner = prepareGradleRunnerFromTemplate("customizedInvalidGroupValue.gradle");
GradleRunner runner = prepareGradleRunnerFromTemplate("customizedInvalidGroupValue.gradle", "build", ZIP_PUBLISH_TASK);
Exception e = assertThrows(UnexpectedBuildFailure.class, runner::build);
assertTrue(
e.getMessage().contains("Invalid publication 'pluginZip': groupId ( ) is not a valid Maven identifier ([A-Za-z0-9_\\-.]+).")
);
}

private GradleRunner prepareGradleRunnerFromTemplate(String templateName) throws IOException, URISyntaxException {
/**
* This test verifies that use of the pluginZip does not clash with other maven publication plugins.
* It covers the case when user calls the "publishToMavenLocal" task.
*/
@Test
public void publishToMavenLocal() throws IOException, URISyntaxException, XmlPullParserException {
// By default, the "publishToMavenLocal" publishes artifacts to a local m2 repo, typically
// found in `~/.m2/repository`. But this is not practical for this unit test at all. We need to point
// the 'maven-publish' plugin to a custom m2 repo located in temporary directory associated with this
// test case instead.
//
// According to Gradle documentation this should be possible by proper configuration of the publishing
// task (https://docs.gradle.org/current/userguide/publishing_maven.html#publishing_maven:install).
// But for some reason this never worked as expected and artifacts created during this test case
// were always pushed into the default local m2 repository (ie: `~/.m2/repository`).
// The only workaround that seems to work is to pass "-Dmaven.repo.local" property via runner argument.
// (Kudos to: https://stackoverflow.com/questions/72265294/gradle-publishtomavenlocal-specify-custom-directory)
//
// The temporary directory that is used as the local m2 repository is created via in task "prepareLocalMVNRepo".
GradleRunner runner = prepareGradleRunnerFromTemplate("publishToMavenLocal.gradle",
String.join(
File.separator,
"-Dmaven.repo.local="+projectDir.getRoot(),
"build",
"local-staging-repo"
),
"build",
"prepareLocalMVNRepo",
"publishToMavenLocal"
);
BuildResult result = runner.build();

assertEquals(SUCCESS, result.task(":" + "build").getOutcome());
assertEquals(SUCCESS, result.task(":" + "publishToMavenLocal").getOutcome());

// Parse the maven file and validate it
MavenXpp3Reader reader = new MavenXpp3Reader();
Model model = reader.read(
new FileReader(
new File(
projectDir.getRoot(),
String.join(
File.separator,
"build",
"local-staging-repo",
"org",
"custom",
"group",
PROJECT_NAME,
"2.0.0.0",
PROJECT_NAME + "-2.0.0.0.pom"
)
)
)
);

// The "publishToMavenLocal" task will run ALL maven publications, hence we can expect the ZIP publication
// present as well: https://docs.gradle.org/current/userguide/publishing_maven.html#publishing_maven:tasks
assertEquals(model.getArtifactId(), PROJECT_NAME);
assertEquals(model.getGroupId(), "org.custom.group");
assertEquals(model.getVersion(), "2.0.0.0");
assertEquals(model.getPackaging(), "zip");

// We have two publications in the build.gradle file, both are "MavenPublication" based.
// Both the mavenJava and pluginZip publications publish to the same location (coordinates) and
// artifacts (the POM file) overwrite each other. However, we can verify that the Zip plugin is
// the last one and "wins" over the mavenJava.
assertEquals(model.getDescription(), "pluginZip publication");
}

/**
* A helper method for use cases
*
* @param templateName The name of the file (from "pluginzip" folder) to use as a build.gradle for the test
* @param gradleArguments Optional CLI parameters to pass into Gradle runner
*/
private GradleRunner prepareGradleRunnerFromTemplate(String templateName, String ... gradleArguments) throws IOException, URISyntaxException {
useTemplateFile(projectDir.newFile("build.gradle"), templateName);
prepareGradleFilesAndSources();

GradleRunner runner = GradleRunner.create()
.forwardOutput()
.withPluginClasspath()
.withArguments("build", ZIP_PUBLISH_TASK)
.withArguments(gradleArguments)
.withProjectDir(projectDir.getRoot());

return runner;
Expand All @@ -246,7 +326,7 @@ private GradleRunner prepareGradleRunnerFromTemplate(String templateName) throws
private void prepareGradleFilesAndSources() throws IOException {
// A dummy "source" file that is processed with bundlePlugin and put into a ZIP artifact file
File bundleFile = new File(projectDir.getRoot(), PROJECT_NAME + "-source.txt");
Path zipFile = Files.createFile(bundleFile.toPath());
Files.createFile(bundleFile.toPath());
// Setting a project name via settings.gradle file
writeString(projectDir.newFile("settings.gradle"), "rootProject.name = '" + PROJECT_NAME + "'");
}
Expand Down
52 changes: 52 additions & 0 deletions buildSrc/src/test/resources/pluginzip/publishToMavenLocal.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
plugins {
// The java-gradle-plugin adds a new publication called 'pluginMaven' that causes some warnings because it
// clashes a bit with other publications defined in this file. If you are running at the --info level then you can
// expect some warning like the following:
// "Multiple publications with coordinates 'org.custom.group:sample-plugin:2.0.0.0' are published to repository 'mavenLocal'."
id 'java-gradle-plugin'

// https://github.com/nebula-plugins/nebula-publishing-plugin#nebulamaven-base-publish
// id 'nebula.maven-base-publish'

id 'maven-publish'
id 'opensearch.pluginzip'
}

group="org.custom.group"
version='2.0.0.0'

// A bundlePlugin task mockup
tasks.register('bundlePlugin', Zip.class) {
archiveFileName = "sample-plugin-${version}.zip"
destinationDirectory = layout.buildDirectory.dir('distributions')
from layout.projectDirectory.file('sample-plugin-source.txt')
}

// A task to prepare directory for a temporary maven local repository
tasks.register('prepareLocalMVNRepo') {
dependsOn ':bundlePlugin'
doFirst {
File localMVNRepo = new File (layout.buildDirectory.get().getAsFile().getPath(), 'local-staging-repo')
System.out.println('Creating temporary folder for mavenLocal repo: '+ localMVNRepo.toString())
System.out.println("Success: " + localMVNRepo.mkdir())
}
}

publishing {
publications {
// Plugin zip publication
pluginZip(MavenPublication) {
pom {
url = 'http://www.example.com/library'
description = 'pluginZip publication'
}
}
// Standard maven publication
mavenJava(MavenPublication) {
pom {
url = 'http://www.example.com/library'
description = 'mavenJava publication'
}
}
}
}
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 91a3fac

Please sign in to comment.