From f45c9bf4e3ec110d62e5464f49f2bcc302e46390 Mon Sep 17 00:00:00 2001 From: Michael Clarke Date: Fri, 2 Jul 2021 16:07:10 +0100 Subject: [PATCH] #272: Ensure New Code Period settings are persisted The New Code Period webservices contain logic that specifically sets the target branch UUID to the main branch of the project if the Community version of Sonarqube is being used, thereby overriding the branch name that's been saved in the new code period's value. The existing Java Agent functionality has been extended to alter the constructors of the relevant New Code services so they override the bundled Edition Provider that specifies the current platform edition as being Community edition, and instead injects a provider that overrides the current distribution as Developer edition, thereby causing the webservice logic to skip applying defaults for branch values. --- .../plugin/CommunityBranchAgent.java | 63 ++++++------ .../CommunityPlatformEditionProvider.java | 37 ++++++++ .../plugin/CommunityBranchAgentTest.java | 95 +++++++++++++++++-- 3 files changed, 152 insertions(+), 43 deletions(-) create mode 100644 src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityPlatformEditionProvider.java diff --git a/src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchAgent.java b/src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchAgent.java index e401ccb20..456a8db45 100644 --- a/src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchAgent.java +++ b/src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchAgent.java @@ -21,10 +21,12 @@ import javassist.CannotCompileException; import javassist.ClassPool; import javassist.CtClass; +import javassist.CtConstructor; import javassist.CtMethod; import javassist.NotFoundException; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; +import org.sonar.core.platform.EditionProvider; import java.io.IOException; import java.lang.instrument.ClassFileTransformer; @@ -46,18 +48,18 @@ private CommunityBranchAgent() { public static void premain(String args, Instrumentation instrumentation) throws UnmodifiableClassException, ClassNotFoundException { LOGGER.info("Loading agent"); - Edition edition = Edition.fromString(args).orElseThrow(() -> new IllegalArgumentException("Invalid/missing agent argument")); + Component component = Component.fromString(args).orElseThrow(() -> new IllegalArgumentException("Invalid/missing agent argument")); - if (edition == Edition.CE) { - redefineEdition(instrumentation, new MethodDetails("org.sonar.core.platform.PlatformEditionProvider", "get", "return java.util.Optional.of(org.sonar.core.platform.EditionProvider.Edition.DEVELOPER);")); - } else if (edition == Edition.WEB) { - redefineEdition(instrumentation, new MethodDetails("org.sonar.server.almsettings.MultipleAlmFeatureProvider", "enabled", "return true;")); + if (component == Component.CE) { + redefineEdition(instrumentation, "org.sonar.core.platform.PlatformEditionProvider", redefineOptionalEditionGetMethod()); + } else if (component == Component.WEB) { + redefineEdition(instrumentation, "org.sonar.server.almsettings.MultipleAlmFeatureProvider", redefineConstructorEditionProviderField(EditionProvider.Edition.ENTERPRISE)); + redefineEdition(instrumentation, "org.sonar.server.newcodeperiod.ws.SetAction", redefineConstructorEditionProviderField(EditionProvider.Edition.DEVELOPER)); + redefineEdition(instrumentation, "org.sonar.server.newcodeperiod.ws.UnsetAction", redefineConstructorEditionProviderField(EditionProvider.Edition.DEVELOPER)); } } - private static void redefineEdition(Instrumentation instrumentation, MethodDetails methodDetails) throws ClassNotFoundException, UnmodifiableClassException { - String targetClassName = methodDetails.getClassName(); - + private static void redefineEdition(Instrumentation instrumentation, String targetClassName, Redefiner redefiner) throws ClassNotFoundException, UnmodifiableClassException { instrumentation.addTransformer(new ClassFileTransformer() { @Override @@ -74,8 +76,8 @@ public byte[] transform(ClassLoader loader, String className, Class classBein try { ClassPool cp = ClassPool.getDefault(); CtClass cc = cp.get(targetClassName); - CtMethod m = cc.getDeclaredMethod(methodDetails.getMethodName()); - m.setBody(methodDetails.getMethodBody()); + + redefiner.redefine(cc); byteCode = cc.toBytecode(); cc.detach(); @@ -91,37 +93,32 @@ public byte[] transform(ClassLoader loader, String className, Class classBein instrumentation.retransformClasses(Class.forName(targetClassName)); } - private static class MethodDetails { - - private final String className; - private final String methodName; - private final String methodBody; - - public MethodDetails(String className, String methodName, String methodBody) { - this.className = className; - this.methodName = methodName; - this.methodBody = methodBody; - } - - public String getClassName() { - return className; - } - public String getMethodName() { - return methodName; - } + private static Redefiner redefineOptionalEditionGetMethod() { + return ctClass -> { + CtMethod ctMethod = ctClass.getDeclaredMethod("get"); + ctMethod.setBody("return java.util.Optional.of(org.sonar.core.platform.EditionProvider.Edition.DEVELOPER);"); + }; + } - public String getMethodBody() { - return methodBody; - } + private static Redefiner redefineConstructorEditionProviderField(EditionProvider.Edition edition) { + return ctClass -> { + CtConstructor ctConstructor = ctClass.getDeclaredConstructors()[0]; + ctConstructor.insertAfter("this.editionProvider = new com.github.mc1arke.sonarqube.plugin.CommunityPlatformEditionProvider(org.sonar.core.platform.EditionProvider.Edition." + edition.name() + ");"); + }; } - private enum Edition { + private enum Component { CE, WEB; - private static Optional fromString(String input) { + private static Optional fromString(String input) { return Arrays.stream(values()).filter(v -> v.name().toLowerCase(Locale.ENGLISH).equals(input)).findFirst(); } } + @FunctionalInterface + private interface Redefiner { + void redefine(CtClass ctClass) throws CannotCompileException, NotFoundException; + } + } diff --git a/src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityPlatformEditionProvider.java b/src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityPlatformEditionProvider.java new file mode 100644 index 000000000..cb90c21d0 --- /dev/null +++ b/src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityPlatformEditionProvider.java @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2021 Michael Clarke + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + */ +package com.github.mc1arke.sonarqube.plugin; + +import org.sonar.core.platform.PlatformEditionProvider; + +import java.util.Optional; + +public class CommunityPlatformEditionProvider extends PlatformEditionProvider { + + private final Edition edition; + + public CommunityPlatformEditionProvider(Edition edition) { + this.edition = edition; + } + + @Override + public Optional get() { + return Optional.of(edition); + } +} diff --git a/src/test/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchAgentTest.java b/src/test/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchAgentTest.java index 7bf5a64fb..f5607b35e 100644 --- a/src/test/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchAgentTest.java +++ b/src/test/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchAgentTest.java @@ -23,7 +23,13 @@ import org.mockito.ArgumentCaptor; import org.sonar.core.platform.EditionProvider; import org.sonar.core.platform.PlatformEditionProvider; +import org.sonar.db.DbClient; +import org.sonar.db.newcodeperiod.NewCodePeriodDao; import org.sonar.server.almsettings.MultipleAlmFeatureProvider; +import org.sonar.server.component.ComponentFinder; +import org.sonar.server.newcodeperiod.ws.SetAction; +import org.sonar.server.newcodeperiod.ws.UnsetAction; +import org.sonar.server.user.UserSession; import java.io.IOException; import java.io.InputStream; @@ -31,11 +37,13 @@ import java.lang.instrument.IllegalClassFormatException; import java.lang.instrument.Instrumentation; import java.lang.instrument.UnmodifiableClassException; +import java.lang.reflect.Field; import java.util.Optional; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; public class CommunityBranchAgentTest { @@ -47,25 +55,92 @@ public void checkErrorThrownIfAgentArgsNotValid() { .hasMessage("Invalid/missing agent argument"); } - @Test - public void checkRedefineForWebLaunchRedefinesTargetClass() throws ReflectiveOperationException, IOException, UnmodifiableClassException, IllegalClassFormatException { + public void checkRedefineForWebLaunchRedefinesMultipleAlmFeatureClass() throws ReflectiveOperationException, IOException, UnmodifiableClassException, IllegalClassFormatException { + CustomClassloader classLoader = new CustomClassloader(); Instrumentation instrumentation = mock(Instrumentation.class); CommunityBranchAgent.premain("web", instrumentation); ArgumentCaptor classFileTransformerArgumentCaptor = ArgumentCaptor.forClass(ClassFileTransformer.class); verify(instrumentation).retransformClasses(MultipleAlmFeatureProvider.class); - verify(instrumentation).addTransformer(classFileTransformerArgumentCaptor.capture()); + verify(instrumentation, times(3)).addTransformer(classFileTransformerArgumentCaptor.capture()); - try (InputStream inputStream = PlatformEditionProvider.class.getResourceAsStream(PlatformEditionProvider.class.getSimpleName())) { + try (InputStream inputStream = MultipleAlmFeatureProvider.class.getResourceAsStream(MultipleAlmFeatureProvider.class.getSimpleName())) { byte[] input = IOUtils.toByteArray(inputStream); - byte[] result = classFileTransformerArgumentCaptor.getValue().transform(getClass().getClassLoader(), MultipleAlmFeatureProvider.class.getName().replaceAll("\\.", "/"), getClass(), getClass().getProtectionDomain(), input); + byte[] result = classFileTransformerArgumentCaptor.getAllValues().get(0).transform(classLoader, MultipleAlmFeatureProvider.class.getName().replaceAll("\\.", "/"), getClass(), getClass().getProtectionDomain(), input); + Class redefined = classLoader.loadClass(MultipleAlmFeatureProvider.class.getName(), result); - CustomClassloader classLoader = new CustomClassloader(); + PlatformEditionProvider platformEditionProvider = mock(PlatformEditionProvider.class); + + Object multipleAlmFeatureProvider = redefined.getConstructor(PlatformEditionProvider.class).newInstance(platformEditionProvider); + Field editionProviderField = redefined.getDeclaredField("editionProvider"); + editionProviderField.setAccessible(true); + + assertThat(((EditionProvider) editionProviderField.get(multipleAlmFeatureProvider)).get()).isEqualTo(Optional.of(EditionProvider.Edition.ENTERPRISE)); + } + } + + @Test + public void checkRedefineForWebLaunchRedefinesSetActionClass() throws ReflectiveOperationException, IOException, UnmodifiableClassException, IllegalClassFormatException { + CustomClassloader classLoader = new CustomClassloader(); + Instrumentation instrumentation = mock(Instrumentation.class); + + CommunityBranchAgent.premain("web", instrumentation); + + ArgumentCaptor classFileTransformerArgumentCaptor = ArgumentCaptor.forClass(ClassFileTransformer.class); + verify(instrumentation).retransformClasses(SetAction.class); + verify(instrumentation, times(3)).addTransformer(classFileTransformerArgumentCaptor.capture()); + + try (InputStream inputStream = SetAction.class.getResourceAsStream(SetAction.class.getSimpleName())) { + byte[] input = IOUtils.toByteArray(inputStream); + byte[] result = classFileTransformerArgumentCaptor.getAllValues().get(1).transform(classLoader, SetAction.class.getName().replaceAll("\\.", "/"), getClass(), getClass().getProtectionDomain(), input); + + Class setActionClass = classLoader.loadClass(SetAction.class.getName(), result); - Class loadedClass = classLoader.loadClass(MultipleAlmFeatureProvider.class.getName(), result); - assertThat(loadedClass.getMethod("enabled").invoke(loadedClass.getConstructor(PlatformEditionProvider.class).newInstance(new PlatformEditionProvider()))).isEqualTo(true); + DbClient dbClient = mock(DbClient.class); + ComponentFinder componentFinder = mock(ComponentFinder.class); + UserSession userSession = mock(UserSession.class); + PlatformEditionProvider platformEditionProvider = mock(PlatformEditionProvider.class); + NewCodePeriodDao newCodePeriodDao = mock(NewCodePeriodDao.class); + + Object setAction = setActionClass.getConstructor(DbClient.class, UserSession.class, ComponentFinder.class, PlatformEditionProvider.class, NewCodePeriodDao.class) + .newInstance(dbClient, userSession, componentFinder, platformEditionProvider, newCodePeriodDao); + + Field editionProviderField = setActionClass.getDeclaredField("editionProvider"); + editionProviderField.setAccessible(true); + assertThat(((EditionProvider) editionProviderField.get(setAction)).get()).isEqualTo(Optional.of(EditionProvider.Edition.DEVELOPER)); + } + } + + @Test + public void checkRedefineForWebLaunchRedefinesUnsetActionClass() throws IOException, UnmodifiableClassException, IllegalClassFormatException, ReflectiveOperationException { + CustomClassloader classLoader = new CustomClassloader(); + + Instrumentation instrumentation = mock(Instrumentation.class); + CommunityBranchAgent.premain("web", instrumentation); + + ArgumentCaptor classFileTransformerArgumentCaptor = ArgumentCaptor.forClass(ClassFileTransformer.class); + verify(instrumentation).retransformClasses(UnsetAction.class); + verify(instrumentation, times(3)).addTransformer(classFileTransformerArgumentCaptor.capture()); + + try (InputStream inputStream = SetAction.class.getResourceAsStream(SetAction.class.getSimpleName())) { + byte[] input = IOUtils.toByteArray(inputStream); + byte[] result = classFileTransformerArgumentCaptor.getAllValues().get(2).transform(classLoader, UnsetAction.class.getName().replaceAll("\\.", "/"), getClass(), getClass().getProtectionDomain(), input); + + Class unsetActionClass = classLoader.loadClass(UnsetAction.class.getName(), result); + DbClient dbClient = mock(DbClient.class); + ComponentFinder componentFinder = mock(ComponentFinder.class); + UserSession userSession = mock(UserSession.class); + PlatformEditionProvider platformEditionProvider = mock(PlatformEditionProvider.class); + NewCodePeriodDao newCodePeriodDao = mock(NewCodePeriodDao.class); + + Object setAction = unsetActionClass.getConstructor(DbClient.class, UserSession.class, ComponentFinder.class, PlatformEditionProvider.class, NewCodePeriodDao.class) + .newInstance(dbClient, userSession, componentFinder, platformEditionProvider, newCodePeriodDao); + + Field editionProviderField = unsetActionClass.getDeclaredField("editionProvider"); + editionProviderField.setAccessible(true); + assertThat(((EditionProvider) editionProviderField.get(setAction)).get()).isEqualTo(Optional.of(EditionProvider.Edition.DEVELOPER)); } } @@ -77,7 +152,7 @@ public void checkRedefineForWebLaunchSkipsNonTargetClass() throws UnmodifiableCl ArgumentCaptor classFileTransformerArgumentCaptor = ArgumentCaptor.forClass(ClassFileTransformer.class); verify(instrumentation).retransformClasses(MultipleAlmFeatureProvider.class); - verify(instrumentation).addTransformer(classFileTransformerArgumentCaptor.capture()); + verify(instrumentation, times(3)).addTransformer(classFileTransformerArgumentCaptor.capture()); byte[] input = new byte[]{1, 2, 3, 4, 5, 6}; byte[] result = classFileTransformerArgumentCaptor.getValue().transform(getClass().getClassLoader(), "com/github/mc1arke/Dummy", getClass(), getClass().getProtectionDomain(), input); @@ -112,7 +187,7 @@ public void checkRedefineForCeLaunchRedefinesTargetClass() throws ReflectiveOper verify(instrumentation).retransformClasses(PlatformEditionProvider.class); verify(instrumentation).addTransformer(classFileTransformerArgumentCaptor.capture()); - try (InputStream inputStream = PlatformEditionProvider.class.getResourceAsStream(PlatformEditionProvider.class.getSimpleName())) { + try (InputStream inputStream = PlatformEditionProvider.class.getResourceAsStream(PlatformEditionProvider.class.getSimpleName() + ".class")) { byte[] input = IOUtils.toByteArray(inputStream); byte[] result = classFileTransformerArgumentCaptor.getValue().transform(getClass().getClassLoader(), PlatformEditionProvider.class.getName().replaceAll("\\.", "/"), getClass(), getClass().getProtectionDomain(), input);