From 48889dba12f20c7e828469f2617e1bb49b6cfe30 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Sun, 8 Sep 2024 03:45:32 +0200 Subject: [PATCH] Extend test for OSGi manifest attributes (#2738) * Extend test for OSGi manifest attributes * Fix test failure for JDK 21 CI build * Use try-with-resources --- gson/pom.xml | 21 +- .../gson/integration/OSGiManifestIT.java | 238 ++++++++++++++++++ .../com/google/gson/regression/OSGiTest.java | 71 ------ pom.xml | 13 +- 4 files changed, 267 insertions(+), 76 deletions(-) create mode 100644 gson/src/test/java/com/google/gson/integration/OSGiManifestIT.java delete mode 100644 gson/src/test/java/com/google/gson/regression/OSGiTest.java diff --git a/gson/pom.xml b/gson/pom.xml index 4a35e97145..c43bae6080 100644 --- a/gson/pom.xml +++ b/gson/pom.xml @@ -69,6 +69,12 @@ 33.3.0-jre test + + com.google.guava + guava + 33.2.1-jre + test + @@ -173,6 +179,19 @@ --illegal-access=deny + + + org.apache.maven.plugins + maven-failsafe-plugin + + + + integration-test + verify + + + + com.coderplus.maven.plugins copy-rename-maven-plugin @@ -271,7 +290,7 @@ maven-jar-plugin - + ${project.build.outputDirectory}/META-INF/MANIFEST.MF diff --git a/gson/src/test/java/com/google/gson/integration/OSGiManifestIT.java b/gson/src/test/java/com/google/gson/integration/OSGiManifestIT.java new file mode 100644 index 0000000000..4b3eea3256 --- /dev/null +++ b/gson/src/test/java/com/google/gson/integration/OSGiManifestIT.java @@ -0,0 +1,238 @@ +/* + * Copyright (C) 2024 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.gson.integration; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; +import static org.junit.Assert.fail; + +import com.google.common.base.Splitter; +import com.google.gson.internal.GsonBuildConfig; +import java.io.IOException; +import java.io.InputStream; +import java.net.URL; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.jar.Attributes; +import java.util.jar.Manifest; +import java.util.stream.Collectors; +import org.junit.Before; +import org.junit.Test; + +/** + * Performs assertions on the generated OSGi manifest attributes. This is an integration test + * ({@code *IT.java}) intended to be run against the final JAR. + * + *

Note: These tests must be run on the command line with {@code mvn clean verify}, running them + * in the IDE will not work because it won't use the generated JAR, and additionally the + * bnd-maven-plugin seems to behave differently in the IDE (at least Eclipse), adding unexpected + * {@code Import-Package} entries for JDK classes (bnd-maven-plugin bug. + */ +public class OSGiManifestIT { + private static class ManifestData { + public final URL url; + public final Manifest manifest; + + public ManifestData(URL url, Manifest manifest) { + this.url = url; + this.manifest = manifest; + } + } + + private static final String GSON_VERSION = GsonBuildConfig.VERSION; + private Attributes manifestAttributes; + + @Before + public void getGsonManifestAttributes() throws Exception { + ManifestData manifestData = findManifest("com.google.gson"); + // Make sure manifest was loaded from final Gson JAR (and not intermediate manifest is used) + assertWithMessage( + "Should load manifest from Gson JAR file; run this test with `mvn clean verify` on" + + " command line and not from IDE") + .that(manifestData.url.toString()) + .endsWith(".jar!/META-INF/MANIFEST.MF"); + + manifestAttributes = manifestData.manifest.getMainAttributes(); + } + + private String getAttribute(String name) { + return manifestAttributes.getValue(name); + } + + @Test + public void testBundleInformation() { + assertThat(getAttribute("Bundle-SymbolicName")).isEqualTo("com.google.gson"); + assertThat(getAttribute("Bundle-Name")).isEqualTo("Gson"); + assertThat(getAttribute("Bundle-License")) + .isEqualTo("\"Apache-2.0\";link=\"https://www.apache.org/licenses/LICENSE-2.0.txt\""); + assertThat(getAttribute("Bundle-Version")) + .isEqualTo(GSON_VERSION.replace("-SNAPSHOT", ".SNAPSHOT")); + } + + @Test + public void testImports() throws Exception { + // Keep only 'major.minor', drop the 'patch' version + String errorProneVersion = + shortenVersionNumber( + findManifest("com.google.errorprone.annotations") + .manifest + .getMainAttributes() + .getValue("Bundle-Version"), + 1); + String nextMajorErrorProneVersion = increaseVersionNumber(errorProneVersion, 0); + String errorProneVersionRange = + "[" + errorProneVersion + "," + nextMajorErrorProneVersion + ")"; + + List imports = splitPackages(getAttribute("Import-Package")); + // If imports contains `java.*`, then either user started from IDE, or IDE rebuilt project while + // Maven build was running, see https://github.com/bndtools/bnd/issues/6258 + if (imports.stream().anyMatch(i -> i.startsWith("java."))) { + fail( + "Test must be run from command line with `mvn clean verify`; additionally make sure your" + + " IDE did not rebuild the project in the meantime"); + } + + assertThat(imports) + .containsExactly( + // Dependency on JDK's sun.misc.Unsafe should be optional + "sun.misc;resolution:=optional", + "com.google.errorprone.annotations;version=\"" + errorProneVersionRange + "\""); + + // Should not contain any import for Gson's own packages, see + // https://github.com/google/gson/pull/2735#issuecomment-2330047410 + for (String importedPackage : imports) { + assertThat(importedPackage).doesNotContain("com.google.gson"); + } + } + + @Test + public void testExports() { + String gsonVersion = GSON_VERSION.replace("-SNAPSHOT", ""); + + List exports = splitPackages(getAttribute("Export-Package")); + // When not running `mvn clean` the exports might differ slightly, see + // https://github.com/bndtools/bnd/issues/6221 + assertWithMessage("Unexpected exports; make sure you are running `mvn clean ...`") + .that(exports) + // Note: This just represents the currently generated exports; especially the `uses` can be + // adjusted if necessary when Gson's implementation changes + .containsExactly( + "com.google.gson;uses:=\"com.google.gson.reflect,com.google.gson.stream\";version=\"" + + gsonVersion + + "\"", + "com.google.gson.annotations;version=\"" + gsonVersion + "\"", + "com.google.gson.reflect;version=\"" + gsonVersion + "\"", + "com.google.gson.stream;uses:=\"com.google.gson\";version=\"" + gsonVersion + "\""); + } + + @Test + public void testRequireCapability() { + // When building with JDK >= 21, the minimum target version is Java 8 + String expectedJavaVersion = Runtime.version().feature() < 21 ? "1.7" : "1.8"; + + // Defines the minimum required Java version + assertThat(getAttribute("Require-Capability")) + .isEqualTo("osgi.ee;filter:=\"(&(osgi.ee=JavaSE)(version=" + expectedJavaVersion + "))\""); + + // Should not define deprecated "Bundle-RequiredExecutionEnvironment" + assertThat(getAttribute("Bundle-RequiredExecutionEnvironment")).isNull(); + } + + private ManifestData findManifest(String bundleName) throws IOException { + List manifestResources = + Collections.list(getClass().getClassLoader().getResources("META-INF/MANIFEST.MF")); + + for (URL manifestResource : manifestResources) { + Manifest manifest; + try (InputStream is = manifestResource.openStream()) { + manifest = new Manifest(is); + } + if (bundleName.equals(manifest.getMainAttributes().getValue("Bundle-SymbolicName"))) { + return new ManifestData(manifestResource, manifest); + } + } + + fail( + "Cannot find " + + bundleName + + " OSGi bundle manifest among: " + + manifestResources + + "\nRun this test with `mvn clean verify` on command line and not from the IDE."); + return null; + } + + /** Splits a list of packages separated by {@code ','}. */ + private List splitPackages(String packagesString) { + List splitPackages = new ArrayList<>(); + int nextSplitStart = 0; + boolean isInQuotes = false; + + for (int i = 0; i < packagesString.length(); i++) { + char c = packagesString.charAt(i); + // Ignore ',' inside quotes + if (c == '"') { + isInQuotes = !isInQuotes; + } else if (c == ',' && !isInQuotes) { + splitPackages.add(packagesString.substring(nextSplitStart, i)); + nextSplitStart = i + 1; // skip past the ',' + } + } + + // Add package behind last ',' + splitPackages.add(packagesString.substring(nextSplitStart)); + return splitPackages; + } + + /** + * Shortens a version number by dropping lower parts. For example {@code 1.2.3 -> 1.2} (when + * {@code keepPosition = 1}). + * + * @param versionString e.g. "1.2.3" + * @param keepPosition position of the version to keep: 0 = major, 1 = minor, ... + * @return shortened version number + */ + private String shortenVersionNumber(String versionString, int keepPosition) { + return Splitter.on('.') + .splitToStream(versionString) + .limit(keepPosition + 1) + .collect(Collectors.joining(".")); + } + + /** + * Increases part of a version number (and drops lower parts). For example {@code 1.2.3 -> 1.3} + * (when {@code position = 1}). + * + * @param versionString e.g. "1.2.3" + * @param position position of the version to increase: 0 = major, 1 = minor, ... + * @return increased version number + */ + private String increaseVersionNumber(String versionString, int position) { + List splitVersion = new ArrayList<>(); + for (String versionPiece : Splitter.on('.').split(versionString)) { + splitVersion.add(Integer.valueOf(versionPiece)); + } + // Drop lower version parts + splitVersion = splitVersion.subList(0, position + 1); + // Increase version number + splitVersion.set(position, splitVersion.get(position) + 1); + + return splitVersion.stream().map(i -> i.toString()).collect(Collectors.joining(".")); + } +} diff --git a/gson/src/test/java/com/google/gson/regression/OSGiTest.java b/gson/src/test/java/com/google/gson/regression/OSGiTest.java deleted file mode 100644 index 475f3e27bb..0000000000 --- a/gson/src/test/java/com/google/gson/regression/OSGiTest.java +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Copyright (C) 2016 Google Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.google.gson.regression; - -import static com.google.common.truth.Truth.assertWithMessage; -import static org.junit.Assert.fail; - -import com.google.common.base.Splitter; -import java.io.IOException; -import java.io.InputStream; -import java.net.URL; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.jar.Manifest; -import org.junit.Test; - -public class OSGiTest { - @Test - public void testComGoogleGsonAnnotationsPackage() throws Exception { - Manifest mf = findManifest("com.google.gson"); - String importPkg = mf.getMainAttributes().getValue("Import-Package"); - assertWithMessage("Import-Package statement is there").that(importPkg).isNotNull(); - assertWithMessage("There should be no com.google.gson.annotations dependency") - .that(importPkg) - .doesNotContain("com.google.gson.annotations"); - } - - @Test - public void testSunMiscImportPackage() throws Exception { - Manifest mf = findManifest("com.google.gson"); - String importPkg = mf.getMainAttributes().getValue("Import-Package"); - assertWithMessage("Import-Package statement is there").that(importPkg).isNotNull(); - for (String dep : Splitter.on(',').split(importPkg)) { - if (dep.contains("sun.misc")) { - assertWithMessage("sun.misc import is optional").that(dep).contains("resolution:=optional"); - return; - } - } - fail("There should be sun.misc dependency, but was: " + importPkg); - } - - private Manifest findManifest(String pkg) throws IOException { - List urls = new ArrayList<>(); - for (URL u : - Collections.list(getClass().getClassLoader().getResources("META-INF/MANIFEST.MF"))) { - InputStream is = u.openStream(); - Manifest mf = new Manifest(is); - is.close(); - if (pkg.equals(mf.getMainAttributes().getValue("Bundle-SymbolicName"))) { - return mf; - } - urls.add(u); - } - fail("Cannot find " + pkg + " OSGi bundle manifest among: " + urls); - return null; - } -} diff --git a/pom.xml b/pom.xml index 5f5f628470..4b1d0220ca 100644 --- a/pom.xml +++ b/pom.xml @@ -339,7 +339,12 @@ org.apache.maven.plugins maven-surefire-plugin 3.5.0 - + + + org.apache.maven.plugins + maven-failsafe-plugin + 3.5.0 + org.apache.maven.plugins maven-jar-plugin @@ -542,9 +547,9 @@ org.apache.maven.plugins maven-compiler-plugin - - -Xlint:all,-options - + + -Xlint:all,-options +