From 412471a98f5503905960893ea9ddac70a7e197a8 Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Tue, 11 Apr 2023 15:14:48 +0800 Subject: [PATCH] Enhance inconsistent version check (#12038) * Enhance inconsistent version check * Fix seperate * Fix load * Fix load * Fix test * Update pom.xml * Print tree * use zip instead * Fix test --- .artifacts | 107 +++++++++++++++ .github/workflows/build-and-test-pr.yml | 6 +- dubbo-common/pom.xml | 33 ----- .../java/org/apache/dubbo/common/Version.java | 128 ++++++++++++------ .../dubbo/common/version/VersionTest.java | 64 ++++++++- .../{version => test-versions/dubbo-common} | 0 .../spring/schema/DubboNamespaceHandler.java | 6 - .../apache/dubbo/remoting/Transporters.java | 7 - .../dubbo/remoting/exchange/Exchangers.java | 7 - .../org/apache/dubbo/dependency/FileTest.java | 39 ++++++ pom.xml | 38 ++++++ 11 files changed, 331 insertions(+), 104 deletions(-) create mode 100644 .artifacts rename dubbo-common/src/test/resources/META-INF/{version => test-versions/dubbo-common} (100%) diff --git a/.artifacts b/.artifacts new file mode 100644 index 00000000000..7ad42729592 --- /dev/null +++ b/.artifacts @@ -0,0 +1,107 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You 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. +# + +# Please add new modules to the end of the list. + +dubbo +dubbo-auth +dubbo-apache-release +dubbo-bom +dubbo-build-tools +dubbo-cluster +dubbo-common +dubbo-compatible +dubbo-compiler +dubbo-config +dubbo-config-api +dubbo-config-spring +dubbo-configcenter +dubbo-configcenter-apollo +dubbo-configcenter-nacos +dubbo-configcenter-zookeeper +dubbo-container +dubbo-container-api +dubbo-container-spring +dubbo-core-spi +dubbo-dependencies +dubbo-dependencies-all +dubbo-dependencies-bom +dubbo-dependencies-zookeeper +dubbo-dependencies-zookeeper-curator5 +dubbo-distribution +dubbo-filter +dubbo-filter-cache +dubbo-filter-validation +dubbo-kubernetes +dubbo-maven-plugin +dubbo-metadata +dubbo-metadata-api +dubbo-metadata-definition-protobuf +dubbo-metadata-processor +dubbo-metadata-report-nacos +dubbo-metadata-report-redis +dubbo-metadata-report-zookeeper +dubbo-metrics +dubbo-metrics-api +dubbo-metrics-default +dubbo-metrics-metadata +dubbo-metrics-prometheus +dubbo-metrics-registry +dubbo-monitor +dubbo-monitor-api +dubbo-monitor-default +dubbo-native +dubbo-native-plugin +dubbo-parent +dubbo-plugin +dubbo-qos +dubbo-qos-api +dubbo-reactive +dubbo-registry +dubbo-registry-api +dubbo-registry-multicast +dubbo-registry-multiple +dubbo-registry-nacos +dubbo-registry-zookeeper +dubbo-remoting +dubbo-remoting-api +dubbo-remoting-http +dubbo-remoting-netty +dubbo-remoting-netty4 +dubbo-remoting-zookeeper +dubbo-remoting-zookeeper-curator5 +dubbo-rpc +dubbo-rpc-api +dubbo-rpc-dubbo +dubbo-rpc-injvm +dubbo-rpc-rest +dubbo-rpc-triple +dubbo-security +dubbo-serialization +dubbo-serialization-api +dubbo-serialization-fastjson2 +dubbo-serialization-hessian2 +dubbo-serialization-jdk +dubbo-spring-boot +dubbo-spring-boot-actuator +dubbo-spring-boot-actuator-compatible +dubbo-spring-boot-autoconfigure +dubbo-spring-boot-autoconfigure-compatible +dubbo-spring-boot-compatible +dubbo-spring-boot-observability-starter +dubbo-spring-boot-starter +dubbo-spring-security +dubbo-xds diff --git a/.github/workflows/build-and-test-pr.yml b/.github/workflows/build-and-test-pr.yml index 72c5fef4cae..853540e4902 100644 --- a/.github/workflows/build-and-test-pr.yml +++ b/.github/workflows/build-and-test-pr.yml @@ -73,7 +73,9 @@ jobs: cd ./dubbo ./mvnw --batch-mode --no-snapshot-updates -e --no-transfer-progress --fail-fast clean source:jar install -Pjacoco,checkstyle -Dmaven.wagon.httpconnectionManager.ttlSeconds=120 -Dmaven.wagon.http.retryHandler.count=5 -Dmaven.test.skip=true -Dmaven.test.skip.exec=true -DembeddedZookeeperPath=${{ github.workspace }}/.tmp/zookeeper - name: "Pack class result" - run: 7z a ${{ github.workspace }}/class.zip */target/classes/* -r + run: | + shopt -s globstar + zip ${{ github.workspace }}/class.zip **/target/classes/* -r - name: "Upload class result" uses: actions/upload-artifact@v3 with: @@ -81,7 +83,7 @@ jobs: path: ${{ github.workspace }}/class.zip - name: "Pack checkstyle file if failure" if: failure() - run: 7z a ${{ github.workspace }}/checkstyle.zip *checkstyle* -r + run: zip ${{ github.workspace }}/checkstyle.zip *checkstyle* -r - name: "Upload checkstyle file if failure" if: failure() uses: actions/upload-artifact@v3 diff --git a/dubbo-common/pom.xml b/dubbo-common/pom.xml index e05a2634bdd..d9413ee7ad7 100644 --- a/dubbo-common/pom.xml +++ b/dubbo-common/pom.xml @@ -101,37 +101,4 @@ javax.annotation-api - - - - - org.apache.maven.plugins - maven-antrun-plugin - - - get-version-infos - compile - - true - - - - - - - - - - - - - run - - - - - - diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/Version.java b/dubbo-common/src/main/java/org/apache/dubbo/common/Version.java index 6b5bb4f279d..ec317703a5a 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/Version.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/Version.java @@ -18,20 +18,18 @@ import org.apache.dubbo.common.logger.ErrorTypeAwareLogger; import org.apache.dubbo.common.logger.LoggerFactory; -import org.apache.dubbo.common.utils.ClassUtils; -import org.apache.dubbo.common.utils.ConfigUtils; import org.apache.dubbo.common.utils.StringUtils; +import java.io.BufferedReader; import java.io.IOException; +import java.io.InputStreamReader; import java.net.URL; +import java.nio.charset.StandardCharsets; import java.security.CodeSource; -import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; import java.util.Map; -import java.util.Optional; -import java.util.Properties; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -64,25 +62,38 @@ public final class Version { private static final Map VERSION2INT = new HashMap(); static { - // check if there's duplicated jar - Version.checkDuplicate(Version.class); - // get dubbo version and last commit id try { - Properties properties = - ConfigUtils.loadProperties(Collections.emptySet(), "META-INF/version"); - - VERSION = Optional.ofNullable(properties.getProperty("revision")) - .filter(StringUtils::isNotBlank) - .orElseGet(() -> getVersion(Version.class, "")); - LATEST_COMMIT_ID = Optional.ofNullable(properties.getProperty("git.commit.id")).orElse(""); + tryLoadVersionFromResource(); + checkDuplicate(); } catch (Throwable e) { logger.warn(COMMON_UNEXPECTED_EXCEPTION, "", "", "continue the old logic, ignore exception " + e.getMessage(), e); + } + if (StringUtils.isEmpty(VERSION)) { VERSION = getVersion(Version.class, ""); + } + if (StringUtils.isEmpty(LATEST_COMMIT_ID)) { LATEST_COMMIT_ID = ""; } } + private static void tryLoadVersionFromResource() throws IOException { + Enumeration configLoader = Version.class.getClassLoader().getResources("META-INF/versions/dubbo-common"); + if (configLoader.hasMoreElements()) { + URL url = configLoader.nextElement(); + try (BufferedReader reader = new BufferedReader(new InputStreamReader(url.openStream(), StandardCharsets.UTF_8))) { + String line; + while ((line = reader.readLine()) != null) { + if (line.startsWith("revision=")) { + VERSION = line.substring("revision=".length()); + } else if (line.startsWith("git.commit.id=")) { + LATEST_COMMIT_ID = line.substring("git.commit.id=".length()); + } + } + } + } + } + private Version() { } @@ -266,48 +277,75 @@ private static String getFromFile(String file) { return file; } - public static void checkDuplicate(Class cls, boolean failOnError) { - checkDuplicate(cls.getName().replace('.', '/') + ".class", failOnError); + private static void checkDuplicate() { + try { + checkArtifacts(loadArtifactIds()); + } catch (Throwable e) { + logger.error(COMMON_UNEXPECTED_EXCEPTION, "", "", e.getMessage(), e); + } } - public static void checkDuplicate(Class cls) { - checkDuplicate(cls, false); + private static void checkArtifacts(Set artifactIds) throws IOException { + if (!artifactIds.isEmpty()) { + for (String artifactId : artifactIds) { + checkArtifact(artifactId); + } + } } - public static void checkDuplicate(String path, boolean failOnError) { - try { - // search in caller's classloader - Set files = getResources(path); - // duplicated jar is found - if (files.size() > 1) { - String error = "Duplicate class " + path + " in " + files.size() + " jar " + files; - if (failOnError) { - throw new IllegalStateException(error); - } else { - logger.error(COMMON_UNEXPECTED_EXCEPTION, "", "", error); + private static void checkArtifact(String artifactId) throws IOException { + Enumeration artifactEnumeration = Version.class.getClassLoader().getResources("META-INF/versions/" + artifactId); + while (artifactEnumeration.hasMoreElements()) { + URL url = artifactEnumeration.nextElement(); + try (BufferedReader reader = new BufferedReader(new InputStreamReader(url.openStream(), StandardCharsets.UTF_8))) { + String line; + while ((line = reader.readLine()) != null) { + if (line.startsWith("#")) { + continue; + } + String[] artifactInfo = line.split("="); + if (artifactInfo.length == 2) { + String key = artifactInfo[0]; + String value = artifactInfo[1]; + checkVersion(artifactId, url, key, value); + } } } - } catch (Throwable e) { - logger.error(COMMON_UNEXPECTED_EXCEPTION, "", "", e.getMessage(), e); } } - /** - * search resources in caller's classloader - */ - private static Set getResources(String path) throws IOException { - Enumeration urls = ClassUtils.getCallerClassLoader(Version.class).getResources(path); - Set files = new HashSet(); - while (urls.hasMoreElements()) { - URL url = urls.nextElement(); - if (url != null) { - String file = url.getFile(); - if (StringUtils.isNotEmpty(file)) { - files.add(file); + private static void checkVersion(String artifactId, URL url, String key, String value) { + if ("revision".equals(key) && !value.equals(VERSION)) { + String error = "Inconsistent version " + value + " found in " + artifactId + " from " + url.getPath() + ", " + + "expected dubbo-common version is " + VERSION; + logger.error(COMMON_UNEXPECTED_EXCEPTION, "", "", error); + } + if ("git.commit.id".equals(key) && !value.equals(LATEST_COMMIT_ID)) { + String error = "Inconsistent git build commit id " + value + " found in " + artifactId + " from " + url.getPath() + ", " + + "expected dubbo-common version is " + LATEST_COMMIT_ID; + logger.error(COMMON_UNEXPECTED_EXCEPTION, "", "", error); + } + } + + private static Set loadArtifactIds() throws IOException { + Enumeration artifactsEnumeration = Version.class.getClassLoader().getResources("META-INF/versions/.artifacts"); + Set artifactIds = new HashSet<>(); + while (artifactsEnumeration.hasMoreElements()) { + URL url = artifactsEnumeration.nextElement(); + try (BufferedReader reader = new BufferedReader(new InputStreamReader(url.openStream(), StandardCharsets.UTF_8))) { + String line; + while ((line = reader.readLine()) != null) { + if (line.startsWith("#")) { + continue; + } + if (StringUtils.isEmpty(line)) { + continue; + } + artifactIds.add(line); } } } - return files; + return artifactIds; } } diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/version/VersionTest.java b/dubbo-common/src/test/java/org/apache/dubbo/common/version/VersionTest.java index 0c6604e74d9..f9781e44718 100644 --- a/dubbo-common/src/test/java/org/apache/dubbo/common/version/VersionTest.java +++ b/dubbo-common/src/test/java/org/apache/dubbo/common/version/VersionTest.java @@ -22,6 +22,12 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import java.io.FileInputStream; +import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.net.URL; +import java.util.Enumeration; + class VersionTest { @Test @@ -93,12 +99,62 @@ void testIsFramework263OrHigher() { } @Test - void testGetVersion() { - Assertions.assertEquals("1.0.0", Version.getVersion()); + void testGetVersion() throws ClassNotFoundException, NoSuchMethodException, InvocationTargetException, IllegalAccessException { + Class versionClass = reloadVersionClass(); + Assertions.assertEquals("1.0.0", versionClass.getDeclaredMethod("getVersion").invoke(null)); + } + + private static Class reloadVersionClass() throws ClassNotFoundException { + ClassLoader originClassLoader = Thread.currentThread().getContextClassLoader(); + ClassLoader classLoader = new ClassLoader(originClassLoader) { + @Override + public Class loadClass(String name) throws ClassNotFoundException { + if (name.equals("org.apache.dubbo.common.Version")) { + return findClass(name); + } + return super.loadClass(name); + } + + @Override + protected Class findClass(String name) throws ClassNotFoundException { + try { + byte[] bytes = loadClassData(name); + return defineClass(name, bytes, 0, bytes.length); + } catch (Exception e) { + return getParent().loadClass(name); + } + } + + + public byte[] loadClassData(String className) throws IOException { + className = className.replaceAll("\\.", "/"); + String path = Version.class.getProtectionDomain().getCodeSource().getLocation().getPath() + className + ".class"; + FileInputStream fileInputStream; + byte[] classBytes; + fileInputStream = new FileInputStream(path); + int length = fileInputStream.available(); + classBytes = new byte[length]; + fileInputStream.read(classBytes); + fileInputStream.close(); + return classBytes; + } + + @Override + public Enumeration getResources(String name) throws IOException { + + if (name.equals("META-INF/versions/dubbo-common")) { + return super.getResources("META-INF/test-versions/dubbo-common"); + } + return super.getResources(name); + } + }; + Class versionClass = classLoader.loadClass("org.apache.dubbo.common.Version"); + return versionClass; } @Test - void testGetLastCommitId() { - Assertions.assertEquals("82a29fcd674216fe9bea10b6efef3196929dd7ca", Version.getLastCommitId()); + void testGetLastCommitId() throws NoSuchMethodException, ClassNotFoundException, InvocationTargetException, IllegalAccessException { + Class versionClass = reloadVersionClass(); + Assertions.assertEquals("82a29fcd674216fe9bea10b6efef3196929dd7ca", versionClass.getDeclaredMethod("getLastCommitId").invoke(null)); } } diff --git a/dubbo-common/src/test/resources/META-INF/version b/dubbo-common/src/test/resources/META-INF/test-versions/dubbo-common similarity index 100% rename from dubbo-common/src/test/resources/META-INF/version rename to dubbo-common/src/test/resources/META-INF/test-versions/dubbo-common diff --git a/dubbo-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/schema/DubboNamespaceHandler.java b/dubbo-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/schema/DubboNamespaceHandler.java index 10bb9b652d7..1cf9bec517d 100644 --- a/dubbo-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/schema/DubboNamespaceHandler.java +++ b/dubbo-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/schema/DubboNamespaceHandler.java @@ -16,7 +16,6 @@ */ package org.apache.dubbo.config.spring.schema; -import org.apache.dubbo.common.Version; import org.apache.dubbo.config.ApplicationConfig; import org.apache.dubbo.config.ConsumerConfig; import org.apache.dubbo.config.MetadataReportConfig; @@ -47,11 +46,6 @@ * @export */ public class DubboNamespaceHandler extends NamespaceHandlerSupport implements ConfigurableSourceBeanMetadataElement { - - static { - Version.checkDuplicate(DubboNamespaceHandler.class); - } - @Override public void init() { registerBeanDefinitionParser("application", new DubboBeanDefinitionParser(ApplicationConfig.class)); diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/Transporters.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/Transporters.java index 97062e8d5a2..d8dbb4ee45a 100644 --- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/Transporters.java +++ b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/Transporters.java @@ -17,7 +17,6 @@ package org.apache.dubbo.remoting; import org.apache.dubbo.common.URL; -import org.apache.dubbo.common.Version; import org.apache.dubbo.remoting.transport.ChannelHandlerAdapter; import org.apache.dubbo.remoting.transport.ChannelHandlerDispatcher; @@ -26,12 +25,6 @@ */ public class Transporters { - static { - // check duplicate jar package - Version.checkDuplicate(Transporters.class); - Version.checkDuplicate(RemotingException.class); - } - private Transporters() { } diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/Exchangers.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/Exchangers.java index 950b06ac489..c8f9b9da149 100644 --- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/Exchangers.java +++ b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/Exchangers.java @@ -17,7 +17,6 @@ package org.apache.dubbo.remoting.exchange; import org.apache.dubbo.common.URL; -import org.apache.dubbo.common.Version; import org.apache.dubbo.remoting.ChannelHandler; import org.apache.dubbo.remoting.Constants; import org.apache.dubbo.remoting.RemotingException; @@ -29,12 +28,6 @@ * Exchanger facade. (API, Static, ThreadSafe) */ public class Exchangers { - - static { - // check duplicate jar package - Version.checkDuplicate(Exchangers.class); - } - private Exchangers() { } diff --git a/dubbo-test/dubbo-test-modules/src/test/java/org/apache/dubbo/dependency/FileTest.java b/dubbo-test/dubbo-test-modules/src/test/java/org/apache/dubbo/dependency/FileTest.java index 4f912e30014..c8038141a93 100644 --- a/dubbo-test/dubbo-test-modules/src/test/java/org/apache/dubbo/dependency/FileTest.java +++ b/dubbo-test/dubbo-test-modules/src/test/java/org/apache/dubbo/dependency/FileTest.java @@ -17,6 +17,7 @@ package org.apache.dubbo.dependency; import org.apache.commons.io.FileUtils; +import org.apache.commons.io.IOUtils; import org.dom4j.Document; import org.dom4j.DocumentException; import org.dom4j.Element; @@ -39,6 +40,7 @@ class FileTest { private final static List ignoredModules = new LinkedList<>(); + private final static List ignoredArtifacts = new LinkedList<>(); private final static List ignoredModulesInDubboAll = new LinkedList<>(); static { @@ -49,6 +51,9 @@ class FileTest { ignoredModules.add(Pattern.compile("dubbo-core-spi")); ignoredModules.add(Pattern.compile("dubbo-demo.*")); + ignoredArtifacts.add(Pattern.compile("dubbo-demo.*")); + ignoredArtifacts.add(Pattern.compile("dubbo-test.*")); + ignoredModulesInDubboAll.add(Pattern.compile("dubbo")); ignoredModulesInDubboAll.add(Pattern.compile("dubbo-bom")); ignoredModulesInDubboAll.add(Pattern.compile("dubbo-compiler")); @@ -99,6 +104,40 @@ void checkDubboBom() throws DocumentException { Assertions.assertTrue(expectedArtifactIds.isEmpty(), "Newly created modules must be added to dubbo-bom. Found modules: " + expectedArtifactIds); } + @Test + void checkArtifacts() throws DocumentException, IOException { + File baseFile = getBaseFile(); + + List poms = new LinkedList<>(); + readPoms(baseFile, poms); + + SAXReader reader = new SAXReader(); + + List artifactIds = poms.stream() + .map(f -> { + try { + return reader.read(f); + } catch (DocumentException e) { + throw new RuntimeException(e); + } + }) + .map(Document::getRootElement) + .map(doc -> doc.elementText("artifactId")) + .sorted() + .collect(Collectors.toList()); + + List artifactIdsInRoot = IOUtils.readLines( + this.getClass().getClassLoader().getResource("META-INF/versions/.artifacts").openStream(), + StandardCharsets.UTF_8); + artifactIdsInRoot.removeIf(s -> s.startsWith("#")); + + List expectedArtifactIds = new LinkedList<>(artifactIds); + expectedArtifactIds.removeAll(artifactIdsInRoot); + expectedArtifactIds.removeIf(artifactId -> ignoredArtifacts.stream().anyMatch(pattern -> pattern.matcher(artifactId).matches())); + + Assertions.assertTrue(expectedArtifactIds.isEmpty(), "Newly created modules must be added to .artifacts (in project root). Found modules: " + expectedArtifactIds); + } + @Test void checkDubboDependenciesAll() throws DocumentException { File baseFile = getBaseFile(); diff --git a/pom.xml b/pom.xml index 96b92e6416c..88c1b749ab4 100644 --- a/pom.xml +++ b/pom.xml @@ -615,6 +615,14 @@ LICENSE + + ${maven.multiModuleProjectDirectory} + META-INF/versions + false + + .artifacts + + @@ -701,6 +709,36 @@ ${file_encoding} + + org.apache.maven.plugins + maven-antrun-plugin + + + get-version-infos + compile + + true + + + + + + + + + + + + + + + run + + + + org.apache.maven.plugins maven-release-plugin