AMBARI-20262 - Startup Annotation Scanning Takes Too Long (jonathanhurley)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/e1e28c6b Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/e1e28c6b Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/e1e28c6b Branch: refs/heads/branch-feature-AMBARI-12556 Commit: e1e28c6beb474a0a66a478c50d95889206929c20 Parents: 453a594 Author: Jonathan Hurley <[email protected]> Authored: Wed Mar 1 15:50:12 2017 -0500 Committer: Jonathan Hurley <[email protected]> Committed: Thu Mar 2 10:58:13 2017 -0500 ---------------------------------------------------------------------- .../ambari/server/audit/AuditLoggerModule.java | 13 ++- .../server/cleanup/ClasspathScannerUtils.java | 19 +++-- .../ambari/server/cleanup/CleanupModule.java | 13 ++- .../server/controller/ControllerModule.java | 89 +++++++++++--------- .../UpgradeResourceProviderHDP22Test.java | 34 ++------ .../internal/UpgradeResourceProviderTest.java | 26 ++---- .../internal/WidgetResourceProviderTest.java | 27 +++--- .../server/orm/InMemoryDefaultTestModule.java | 8 +- 8 files changed, 105 insertions(+), 124 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/e1e28c6b/ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerModule.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerModule.java b/ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerModule.java index d8b8466..b325dcf 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerModule.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerModule.java @@ -27,6 +27,8 @@ import org.apache.ambari.server.audit.request.RequestAuditLogger; import org.apache.ambari.server.audit.request.RequestAuditLoggerImpl; import org.apache.ambari.server.audit.request.eventcreator.RequestAuditEventCreator; import org.apache.ambari.server.cleanup.ClasspathScannerUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.inject.AbstractModule; import com.google.inject.Scopes; @@ -34,14 +36,15 @@ import com.google.inject.multibindings.Multibinder; import com.google.inject.name.Names; public class AuditLoggerModule extends AbstractModule { + private static final Logger LOG = LoggerFactory.getLogger(AuditLoggerModule.class); /** * Selectors identifying objects to be bound. * * @return a list with interface and annotation types */ - protected List<Class> getSelectors() { - List<Class> selectorList = new ArrayList<>(); + protected List<Class<?>> getSelectors() { + List<Class<?>> selectorList = new ArrayList<>(); selectorList.add(RequestAuditEventCreator.class); return selectorList; } @@ -51,7 +54,7 @@ public class AuditLoggerModule extends AbstractModule { * * @return a list with types to be left out from dynamic bindings */ - protected List<Class> getExclusions() { + protected List<Class<?>> getExclusions() { return Collections.emptyList(); } @@ -73,8 +76,10 @@ public class AuditLoggerModule extends AbstractModule { // binding for audit event creators Multibinder<RequestAuditEventCreator> multiBinder = Multibinder.newSetBinder(binder(), RequestAuditEventCreator.class); - Set<Class> bindingSet = ClasspathScannerUtils.findOnClassPath(getPackageToScan(), getExclusions(), getSelectors()); + Set<Class<?>> bindingSet = ClasspathScannerUtils.findOnClassPath(getPackageToScan(), getExclusions(), getSelectors()); + for (Class clazz : bindingSet) { + LOG.info("Binding audit event creator {}", clazz); multiBinder.addBinding().to(clazz).in(Scopes.SINGLETON); } http://git-wip-us.apache.org/repos/asf/ambari/blob/e1e28c6b/ambari-server/src/main/java/org/apache/ambari/server/cleanup/ClasspathScannerUtils.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/cleanup/ClasspathScannerUtils.java b/ambari-server/src/main/java/org/apache/ambari/server/cleanup/ClasspathScannerUtils.java index 01487f0..a1a42bc 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/cleanup/ClasspathScannerUtils.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/cleanup/ClasspathScannerUtils.java @@ -20,7 +20,7 @@ package org.apache.ambari.server.cleanup; import java.io.IOException; import java.lang.annotation.Annotation; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -45,15 +45,16 @@ public class ClasspathScannerUtils { * @param selectors a list with annotation and interface classes that identify classes to be found (lookup criteria) * @return a list of classes from the classpath that match the lookup criteria */ - public static Set<Class> findOnClassPath(String packageName, List<Class> exclusions, List<Class> selectors) { + public static Set<Class<?>> findOnClassPath(String packageName, List<Class<?>> exclusions, + List<Class<?>> selectors) { - Set<Class> bindingSet = new HashSet<>(); + Set<Class<?>> bindingSet = new LinkedHashSet<>(); try { ClassPath classpath = ClassPath.from(ClasspathScannerUtils.class.getClassLoader()); LOGGER.info("Checking package [{}] for binding candidates.", packageName); for (ClassPath.ClassInfo classInfo : classpath.getTopLevelClassesRecursive(packageName)) { - Class candidate = classInfo.load(); + Class<?> candidate = classInfo.load(); if (exclusions.contains(candidate)) { LOGGER.debug("Candidate [{}] is excluded excluded.", candidate); @@ -61,7 +62,7 @@ public class ClasspathScannerUtils { } if (isEligible(candidate, selectors)) { - LOGGER.info("Found class [{}]", candidate); + LOGGER.debug("Found class [{}]", candidate); bindingSet.add(candidate); } else { LOGGER.debug("Candidate [{}] doesn't match.", candidate); @@ -82,7 +83,7 @@ public class ClasspathScannerUtils { * @param candidate the type to be checked * @return true if the class matches, false otherwise */ - private static boolean isEligible(Class candidate, List<Class> selectors) { + private static boolean isEligible(Class<?> candidate, List<Class<?>> selectors) { return checkSubClasses(candidate, selectors) || checkAnnotations(candidate, selectors); } @@ -92,11 +93,11 @@ public class ClasspathScannerUtils { * @param candidate the type to be checked * @return true if the candidate has annotations listed in the selection criteria, false otherwise */ - private static boolean checkAnnotations(Class candidate, List<Class> selectors) { + private static boolean checkAnnotations(Class<?> candidate, List<Class<?>> selectors) { LOGGER.debug("Checking annotations for: [{}]", candidate); boolean ret = false; for (Annotation candidateAnn : candidate.getDeclaredAnnotations()) { - if (selectors.contains(candidateAnn)) { + if (selectors.contains(candidateAnn.annotationType())) { ret = true; break; } @@ -110,7 +111,7 @@ public class ClasspathScannerUtils { * @param candidate the type to be checked * @return true if the candidate implements interfaces listed in the selection criteria, false otherwise */ - private static boolean checkSubClasses(Class candidate, List<Class> selectors) { + private static boolean checkSubClasses(Class<?> candidate, List<Class<?>> selectors) { boolean ret = false; LOGGER.debug("Checking interfaces for: [{}]", candidate); List interfaces = ClassUtils.getAllInterfaces(candidate); http://git-wip-us.apache.org/repos/asf/ambari/blob/e1e28c6b/ambari-server/src/main/java/org/apache/ambari/server/cleanup/CleanupModule.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/cleanup/CleanupModule.java b/ambari-server/src/main/java/org/apache/ambari/server/cleanup/CleanupModule.java index d2a7583..da90cde 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/cleanup/CleanupModule.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/cleanup/CleanupModule.java @@ -23,6 +23,8 @@ import java.util.List; import java.util.Set; import org.apache.ambari.server.orm.dao.Cleanable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.inject.AbstractModule; import com.google.inject.Scopes; @@ -32,14 +34,15 @@ import com.google.inject.multibindings.Multibinder; * Configuration module for the cleanup framework. */ public class CleanupModule extends AbstractModule { + private static final Logger LOG = LoggerFactory.getLogger(CleanupModule.class); /** * Selectors identifying objects to be bound. * * @return a list with interface and annotation types */ - protected List<Class> getSelectors() { - List<Class> selectorList = new ArrayList<>(); + protected List<Class<?>> getSelectors() { + List<Class<?>> selectorList = new ArrayList<>(); selectorList.add(Cleanable.class); return selectorList; } @@ -49,7 +52,7 @@ public class CleanupModule extends AbstractModule { * * @return a list with types to be left out from dynamic bindings */ - protected List<Class> getExclusions() { + protected List<Class<?>> getExclusions() { return Collections.emptyList(); } @@ -67,10 +70,12 @@ public class CleanupModule extends AbstractModule { protected void configure() { Multibinder<Cleanable> multiBinder = Multibinder.newSetBinder(binder(), Cleanable.class); - Set<Class> bindingSet = ClasspathScannerUtils.findOnClassPath(getPackageToScan(), getExclusions(), getSelectors()); + Set<Class<?>> bindingSet = ClasspathScannerUtils.findOnClassPath(getPackageToScan(), getExclusions(), getSelectors()); for (Class clazz : bindingSet) { + LOG.info("Binding cleaner {}", clazz); multiBinder.addBinding().to(clazz).in(Scopes.SINGLETON); } + bind(CleanupServiceImpl.class).in(Scopes.SINGLETON); } } http://git-wip-us.apache.org/repos/asf/ambari/blob/e1e28c6b/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java index 8646e51..482d602 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java @@ -34,10 +34,9 @@ import static org.eclipse.persistence.config.PersistenceUnitProperties.NON_JTA_D import static org.eclipse.persistence.config.PersistenceUnitProperties.THROW_EXCEPTIONS; import java.beans.PropertyVetoException; -import java.lang.annotation.Annotation; import java.security.SecureRandom; import java.text.MessageFormat; -import java.util.Arrays; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Properties; @@ -57,6 +56,7 @@ import org.apache.ambari.server.actionmanager.StageFactoryImpl; import org.apache.ambari.server.checks.AbstractCheckDescriptor; import org.apache.ambari.server.checks.DatabaseConsistencyCheckHelper; import org.apache.ambari.server.checks.UpgradeCheckRegistry; +import org.apache.ambari.server.cleanup.ClasspathScannerUtils; import org.apache.ambari.server.configuration.Configuration; import org.apache.ambari.server.configuration.Configuration.ConnectionPoolType; import org.apache.ambari.server.configuration.Configuration.DatabaseType; @@ -91,6 +91,7 @@ import org.apache.ambari.server.metrics.system.MetricsService; import org.apache.ambari.server.metrics.system.impl.MetricsServiceImpl; import org.apache.ambari.server.notifications.DispatchFactory; import org.apache.ambari.server.notifications.NotificationDispatcher; +import org.apache.ambari.server.notifications.dispatchers.AlertScriptDispatcher; import org.apache.ambari.server.notifications.dispatchers.AmbariSNMPDispatcher; import org.apache.ambari.server.notifications.dispatchers.SNMPDispatcher; import org.apache.ambari.server.orm.DBAccessor; @@ -153,13 +154,13 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider; -import org.springframework.core.type.filter.AnnotationTypeFilter; import org.springframework.core.type.filter.AssignableTypeFilter; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.crypto.password.StandardPasswordEncoder; import org.springframework.util.ClassUtils; import org.springframework.web.filter.DelegatingFilterProxy; +import com.google.common.reflect.ClassPath; import com.google.common.util.concurrent.ServiceManager; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -503,50 +504,46 @@ public class ControllerModule extends AbstractModule { * A second example of where this is needed is when classes require static * members that are available via injection. * <p/> - * If {@code beanDefinitions} is empty or null this will scan + * Although Spring has a nicer API for performing this search, it's dreadfully + * slow on annotation processing. This class will use + * {@link ClasspathScannerUtils} which in turn uses {@link ClassPath} to + * perform the scan. + * <p/> + * If {@code matchedClasses} is null this will scan * {@code org.apache.ambari.server} (currently) for any {@link EagerSingleton} * or {@link StaticallyInject} or {@link AmbariService} instances. * - * @param beanDefinitions the set of bean definitions. If it is empty or - * {@code null} scan will occur. - * @return the set of bean definitions that was found during scan if - * {@code beanDefinitions} was null or empty. Else original - * {@code beanDefinitions} will be returned. + * @param matchedClasses + * the set of previously found classes, or {@code null} to invoke + * scanning. + * @return the set of classes that was found. */ - // Method is protected and returns a set of bean definitions for testing convenience. @SuppressWarnings("unchecked") - protected Set<BeanDefinition> bindByAnnotation(Set<BeanDefinition> beanDefinitions) { - List<Class<? extends Annotation>> classes = Arrays.asList( - EagerSingleton.class, StaticallyInject.class, AmbariService.class); - - if (null == beanDefinitions || beanDefinitions.size() == 0) { - ClassPathScanningCandidateComponentProvider scanner = - new ClassPathScanningCandidateComponentProvider(false); - - // match only singletons that are eager listeners - for (Class<? extends Annotation> cls : classes) { - scanner.addIncludeFilter(new AnnotationTypeFilter(cls)); + protected Set<Class<?>> bindByAnnotation(Set<Class<?>> matchedClasses) { + // only search if necessary + if (null == matchedClasses) { + List<Class<?>> classes = new ArrayList<>(); + classes.add(EagerSingleton.class); + classes.add(StaticallyInject.class); + classes.add(AmbariService.class); + + LOG.info("Searching package {} for annotations matching {}", AMBARI_PACKAGE, classes); + + matchedClasses = ClasspathScannerUtils.findOnClassPath(AMBARI_PACKAGE, + new ArrayList<Class<?>>(), classes); + + if (null == matchedClasses || matchedClasses.size() == 0) { + LOG.warn("No instances of {} found to register", classes); + return matchedClasses; } - - beanDefinitions = scanner.findCandidateComponents(AMBARI_PACKAGE); - } - - if (null == beanDefinitions || beanDefinitions.size() == 0) { - LOG.warn("No instances of {} found to register", classes); - return beanDefinitions; } - Set<com.google.common.util.concurrent.Service> services = - new HashSet<com.google.common.util.concurrent.Service>(); - - for (BeanDefinition beanDefinition : beanDefinitions) { - String className = beanDefinition.getBeanClassName(); - Class<?> clazz = ClassUtils.resolveClassName(className, - ClassUtils.getDefaultClassLoader()); + Set<com.google.common.util.concurrent.Service> services = new HashSet<>(); + for (Class<?> clazz : matchedClasses) { if (null != clazz.getAnnotation(EagerSingleton.class)) { bind(clazz).asEagerSingleton(); - LOG.debug("Binding singleton {} eagerly", clazz); + LOG.debug("Eagerly binding singleton {}", clazz); } if (null != clazz.getAnnotation(StaticallyInject.class)) { @@ -562,7 +559,7 @@ public class ControllerModule extends AbstractModule { "Unable to register service {0} because it is not a Service which can be scheduled", clazz); - LOG.warn(message); + LOG.error(message); throw new RuntimeException(message); } @@ -572,7 +569,7 @@ public class ControllerModule extends AbstractModule { service = (com.google.common.util.concurrent.Service) clazz.newInstance(); bind((Class<com.google.common.util.concurrent.Service>) clazz).toInstance(service); services.add(service); - LOG.debug("Registering service {} ", clazz); + LOG.info("Registering service {} ", clazz); } catch (Exception exception) { LOG.error("Unable to register {} as a service", clazz, exception); throw new RuntimeException(exception); @@ -583,7 +580,7 @@ public class ControllerModule extends AbstractModule { ServiceManager manager = new ServiceManager(services); bind(ServiceManager.class).toInstance(manager); - return beanDefinitions; + return matchedClasses; } /** @@ -599,6 +596,10 @@ public class ControllerModule extends AbstractModule { bind(DispatchFactory.class).toInstance(dispatchFactory); if (null == beanDefinitions || beanDefinitions.isEmpty()) { + String packageName = AlertScriptDispatcher.class.getPackage().getName(); + LOG.info("Searching package {} for dispatchers matching {}", packageName, + NotificationDispatcher.class); + ClassPathScanningCandidateComponentProvider scanner = new ClassPathScanningCandidateComponentProvider(false); @@ -608,7 +609,7 @@ public class ControllerModule extends AbstractModule { scanner.addIncludeFilter(filter); - beanDefinitions = scanner.findCandidateComponents("org.apache.ambari.server.notifications.dispatchers"); + beanDefinitions = scanner.findCandidateComponents(packageName); } // no dispatchers is a problem @@ -659,13 +660,17 @@ public class ControllerModule extends AbstractModule { bind(UpgradeCheckRegistry.class).toInstance(registry); if (null == beanDefinitions || beanDefinitions.isEmpty()) { + String packageName = AbstractCheckDescriptor.class.getPackage().getName(); + LOG.info("Searching package {} for classes matching {}", packageName, + AbstractCheckDescriptor.class); + ClassPathScanningCandidateComponentProvider scanner = new ClassPathScanningCandidateComponentProvider(false); // match all implementations of the base check class AssignableTypeFilter filter = new AssignableTypeFilter(AbstractCheckDescriptor.class); scanner.addIncludeFilter(filter); - beanDefinitions = scanner.findCandidateComponents(AbstractCheckDescriptor.class.getPackage().getName()); + beanDefinitions = scanner.findCandidateComponents(packageName); } // no dispatchers is a problem @@ -692,7 +697,7 @@ public class ControllerModule extends AbstractModule { // log the order of the pre-upgrade checks List<AbstractCheckDescriptor> upgradeChecks = registry.getUpgradeChecks(); for (AbstractCheckDescriptor upgradeCheck : upgradeChecks) { - LOG.debug("Registered pre-upgrade check {}", upgradeCheck.getClass()); + LOG.info("Registered pre-upgrade check {}", upgradeCheck.getClass()); } return beanDefinitions; } http://git-wip-us.apache.org/repos/asf/ambari/blob/e1e28c6b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderHDP22Test.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderHDP22Test.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderHDP22Test.java index 40c7e50..36889b2 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderHDP22Test.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderHDP22Test.java @@ -17,19 +17,14 @@ */ package org.apache.ambari.server.controller.internal; -import static org.easymock.EasyMock.anyLong; import static org.easymock.EasyMock.createNiceMock; -import static org.easymock.EasyMock.eq; -import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import java.lang.reflect.Field; -import java.lang.reflect.Method; import java.sql.SQLException; import java.util.Collections; -import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -63,9 +58,7 @@ import org.apache.ambari.server.orm.entities.StackEntity; import org.apache.ambari.server.orm.entities.UpgradeEntity; import org.apache.ambari.server.orm.entities.UpgradeGroupEntity; import org.apache.ambari.server.orm.entities.UpgradeItemEntity; -import org.apache.ambari.server.security.authorization.AuthorizationHelper; -import org.apache.ambari.server.security.authorization.ResourceType; -import org.apache.ambari.server.security.authorization.RoleAuthorization; +import org.apache.ambari.server.security.TestAuthenticationFactory; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; @@ -84,11 +77,7 @@ import org.apache.ambari.server.view.ViewRegistry; import org.junit.After; import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; -import org.powermock.api.easymock.PowerMock; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; +import org.springframework.security.core.context.SecurityContextHolder; import com.google.common.collect.ImmutableMap; import com.google.gson.Gson; @@ -98,9 +87,6 @@ import com.google.inject.Injector; /** * UpgradeResourceDefinition tests. */ -@RunWith(PowerMockRunner.class) -@PrepareForTest({AuthorizationHelper.class}) -@PowerMockIgnore({"javax.management.*", "javax.crypto.*"}) public class UpgradeResourceProviderHDP22Test { private UpgradeDAO upgradeDao = null; @@ -124,6 +110,9 @@ public class UpgradeResourceProviderHDP22Test { @Before public void before() throws Exception { + SecurityContextHolder.getContext().setAuthentication( + TestAuthenticationFactory.createAdministrator()); + // create an injector which will inject the mocks injector = Guice.createInjector(new InMemoryDefaultTestModule()); @@ -174,7 +163,7 @@ public class UpgradeResourceProviderHDP22Test { clusters.addHost("h1"); Host host = clusters.getHost("h1"); - Map<String, String> hostAttributes = new HashMap<String, String>(); + Map<String, String> hostAttributes = new HashMap<>(); hostAttributes.put("os_family", "redhat"); hostAttributes.put("os_release_version", "6.3"); host.setHostAttributes(hostAttributes); @@ -197,13 +186,6 @@ public class UpgradeResourceProviderHDP22Test { StageUtils.setTopologyManager(topologyManager); StageUtils.setConfiguration(injector.getInstance(Configuration.class)); ActionManager.setTopologyManager(topologyManager); - - - Method isAuthorizedMethod = AuthorizationHelper.class.getMethod("isAuthorized", ResourceType.class, Long.class, Set.class); - PowerMock.mockStatic(AuthorizationHelper.class, isAuthorizedMethod); - expect(AuthorizationHelper.isAuthorized(eq(ResourceType.CLUSTER), anyLong(), - eq(EnumSet.of(RoleAuthorization.CLUSTER_UPGRADE_DOWNGRADE_STACK)))).andReturn(true).anyTimes(); - PowerMock.replay(AuthorizationHelper.class); } @After @@ -241,7 +223,7 @@ public class UpgradeResourceProviderHDP22Test { Config config = configFactory.createNew(cluster, "hive-site", configTagVersion1, configTagVersion1Properties, null); cluster.addDesiredConfig("admin", Collections.singleton(config)); - Map<String, Object> requestProps = new HashMap<String, Object>(); + Map<String, Object> requestProps = new HashMap<>(); requestProps.put(UpgradeResourceProvider.UPGRADE_CLUSTER_NAME, "c1"); requestProps.put(UpgradeResourceProvider.UPGRADE_VERSION, "2.2.4.2"); requestProps.put(UpgradeResourceProvider.UPGRADE_SKIP_PREREQUISITE_CHECKS, "true"); @@ -307,7 +289,7 @@ public class UpgradeResourceProviderHDP22Test { // ensure that the latest tag is being used - this is absolutely required // for upgrades - Set<String> roleCommandsThatMustHaveRefresh = new HashSet<String>(); + Set<String> roleCommandsThatMustHaveRefresh = new HashSet<>(); roleCommandsThatMustHaveRefresh.add("SERVICE_CHECK"); roleCommandsThatMustHaveRefresh.add("RESTART"); roleCommandsThatMustHaveRefresh.add("ACTIONEXECUTE"); http://git-wip-us.apache.org/repos/asf/ambari/blob/e1e28c6b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java index 4d609b4..62e55c4 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java @@ -17,9 +17,7 @@ */ package org.apache.ambari.server.controller.internal; -import static org.easymock.EasyMock.anyLong; import static org.easymock.EasyMock.createNiceMock; -import static org.easymock.EasyMock.eq; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; import static org.junit.Assert.assertEquals; @@ -29,11 +27,9 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.lang.reflect.Field; -import java.lang.reflect.Method; import java.sql.SQLException; import java.util.ArrayList; import java.util.Collections; -import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; @@ -82,9 +78,7 @@ import org.apache.ambari.server.orm.entities.StageEntity; import org.apache.ambari.server.orm.entities.UpgradeEntity; import org.apache.ambari.server.orm.entities.UpgradeGroupEntity; import org.apache.ambari.server.orm.entities.UpgradeItemEntity; -import org.apache.ambari.server.security.authorization.AuthorizationHelper; -import org.apache.ambari.server.security.authorization.ResourceType; -import org.apache.ambari.server.security.authorization.RoleAuthorization; +import org.apache.ambari.server.security.TestAuthenticationFactory; import org.apache.ambari.server.serveraction.upgrades.AutoSkipFailedSummaryAction; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; @@ -113,11 +107,7 @@ import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; -import org.powermock.api.easymock.PowerMock; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; +import org.springframework.security.core.context.SecurityContextHolder; import com.google.common.collect.Lists; import com.google.gson.Gson; @@ -133,9 +123,6 @@ import com.google.inject.util.Modules; /** * UpgradeResourceDefinition tests. */ -@RunWith(PowerMockRunner.class) -@PrepareForTest({AuthorizationHelper.class}) -@PowerMockIgnore({"javax.management.*", "javax.crypto.*"}) public class UpgradeResourceProviderTest { private UpgradeDAO upgradeDao = null; @@ -154,6 +141,9 @@ public class UpgradeResourceProviderTest { @Before public void before() throws Exception { + SecurityContextHolder.getContext().setAuthentication( + TestAuthenticationFactory.createAdministrator()); + // setup the config helper for placeholder resolution configHelper = EasyMock.createNiceMock(ConfigHelper.class); @@ -265,12 +255,6 @@ public class UpgradeResourceProviderTest { StageUtils.setConfiguration(injector.getInstance(Configuration.class)); ActionManager.setTopologyManager(topologyManager); EasyMock.replay(injector.getInstance(AuditLogger.class)); - - Method isAuthorizedMethod = AuthorizationHelper.class.getMethod("isAuthorized", ResourceType.class, Long.class, Set.class); - PowerMock.mockStatic(AuthorizationHelper.class, isAuthorizedMethod); - expect(AuthorizationHelper.isAuthorized(eq(ResourceType.CLUSTER), anyLong(), - eq(EnumSet.of(RoleAuthorization.CLUSTER_UPGRADE_DOWNGRADE_STACK)))).andReturn(true).anyTimes(); - PowerMock.replay(AuthorizationHelper.class); } @After http://git-wip-us.apache.org/repos/asf/ambari/blob/e1e28c6b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/WidgetResourceProviderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/WidgetResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/WidgetResourceProviderTest.java index a4f6469..e0434c6 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/WidgetResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/WidgetResourceProviderTest.java @@ -46,6 +46,7 @@ import org.apache.ambari.server.controller.utilities.PropertyHelper; import org.apache.ambari.server.orm.InMemoryDefaultTestModule; import org.apache.ambari.server.orm.dao.WidgetDAO; import org.apache.ambari.server.orm.entities.WidgetEntity; +import org.apache.ambari.server.security.TestAuthenticationFactory; import org.apache.ambari.server.security.encryption.CredentialStoreService; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; @@ -55,11 +56,7 @@ import org.easymock.EasyMock; import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; import org.powermock.api.easymock.PowerMock; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.core.context.SecurityContextHolder; @@ -72,9 +69,6 @@ import com.google.inject.util.Modules; /** * Widget tests */ -@RunWith(PowerMockRunner.class) -@PrepareForTest( {WidgetResourceProvider.class, SecurityContextHolder.class} ) -@PowerMockIgnore( {"javax.management.*"} ) public class WidgetResourceProviderTest { private WidgetDAO dao = null; @@ -82,6 +76,9 @@ public class WidgetResourceProviderTest { @Before public void before() { + SecurityContextHolder.getContext().setAuthentication( + TestAuthenticationFactory.createAdministrator()); + dao = createStrictMock(WidgetDAO.class); m_injector = Guice.createInjector(Modules.override( @@ -230,7 +227,7 @@ public class WidgetResourceProviderTest { WidgetResourceProvider provider = createProvider(amc); - Map<String, Object> requestProps = new HashMap<String, Object>(); + Map<String, Object> requestProps = new HashMap<>(); requestProps.put(WidgetResourceProvider.WIDGET_CLUSTER_NAME_PROPERTY_ID, "c1"); requestProps.put(WidgetResourceProvider.WIDGET_WIDGET_NAME_PROPERTY_ID, "widget name"); requestProps.put(WidgetResourceProvider.WIDGET_WIDGET_TYPE_PROPERTY_ID, "GAUGE"); @@ -285,7 +282,7 @@ public class WidgetResourceProviderTest { replay(amc, clusters, cluster, dao); - Map<String, Object> requestProps = new HashMap<String, Object>(); + Map<String, Object> requestProps = new HashMap<>(); requestProps.put(WidgetResourceProvider.WIDGET_CLUSTER_NAME_PROPERTY_ID, "c1"); requestProps.put(WidgetResourceProvider.WIDGET_WIDGET_NAME_PROPERTY_ID, "widget name"); requestProps.put(WidgetResourceProvider.WIDGET_WIDGET_TYPE_PROPERTY_ID, "GAUGE"); @@ -329,7 +326,7 @@ public class WidgetResourceProviderTest { expect(dao.merge((WidgetEntity) anyObject())).andReturn(entity).anyTimes(); replay(dao); - requestProps = new HashMap<String, Object>(); + requestProps = new HashMap<>(); requestProps.put(WidgetResourceProvider.WIDGET_ID_PROPERTY_ID, "1"); requestProps.put(WidgetResourceProvider.WIDGET_CLUSTER_NAME_PROPERTY_ID, "c1"); requestProps.put(WidgetResourceProvider.WIDGET_WIDGET_NAME_PROPERTY_ID, "widget name2"); @@ -382,7 +379,7 @@ public class WidgetResourceProviderTest { WidgetResourceProvider provider = createProvider(amc); - Map<String, Object> requestProps = new HashMap<String, Object>(); + Map<String, Object> requestProps = new HashMap<>(); requestProps.put(WidgetResourceProvider.WIDGET_CLUSTER_NAME_PROPERTY_ID, "c1"); requestProps.put(WidgetResourceProvider.WIDGET_WIDGET_NAME_PROPERTY_ID, "widget name"); requestProps.put(WidgetResourceProvider.WIDGET_WIDGET_TYPE_PROPERTY_ID, "GAUGE"); @@ -430,14 +427,15 @@ public class WidgetResourceProviderTest { @Test public void testScopePrivilegeCheck() throws Exception { + SecurityContextHolder.getContext().setAuthentication( + TestAuthenticationFactory.createServiceOperator()); + AmbariManagementController amc = createMock(AmbariManagementController.class); Clusters clusters = createMock(Clusters.class); Cluster cluster = createMock(Cluster.class); expect(amc.getClusters()).andReturn(clusters).atLeastOnce(); expect(clusters.getCluster((String) anyObject())).andReturn(cluster).atLeastOnce(); expect(cluster.getClusterId()).andReturn(Long.valueOf(1)).atLeastOnce(); - WidgetResourceProvider widgetResourceProvider = PowerMock.createPartialMock(WidgetResourceProvider.class, "isScopeAllowedForUser"); - PowerMock.expectPrivate(widgetResourceProvider, "isScopeAllowedForUser", "CLUSTER").andReturn(false); Capture<WidgetEntity> entityCapture = EasyMock.newCapture(); dao.create(capture(entityCapture)); @@ -446,7 +444,7 @@ public class WidgetResourceProviderTest { replay(amc, clusters, cluster, dao); PowerMock.replayAll(); - Map<String, Object> requestProps = new HashMap<String, Object>(); + Map<String, Object> requestProps = new HashMap<>(); requestProps.put(WidgetResourceProvider.WIDGET_CLUSTER_NAME_PROPERTY_ID, "c1"); requestProps.put(WidgetResourceProvider.WIDGET_WIDGET_NAME_PROPERTY_ID, "widget name"); requestProps.put(WidgetResourceProvider.WIDGET_WIDGET_TYPE_PROPERTY_ID, "GAUGE"); @@ -456,6 +454,7 @@ public class WidgetResourceProviderTest { Collections.singleton(requestProps), null); try { + WidgetResourceProvider widgetResourceProvider = createProvider(amc); widgetResourceProvider.createResources(request); } catch (AccessDeniedException ex) { //Expected exception http://git-wip-us.apache.org/repos/asf/ambari/blob/e1e28c6b/ambari-server/src/test/java/org/apache/ambari/server/orm/InMemoryDefaultTestModule.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/orm/InMemoryDefaultTestModule.java b/ambari-server/src/test/java/org/apache/ambari/server/orm/InMemoryDefaultTestModule.java index 3090108..f231b60 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/orm/InMemoryDefaultTestModule.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/orm/InMemoryDefaultTestModule.java @@ -50,7 +50,7 @@ public class InMemoryDefaultTestModule extends AbstractModule { private static class BeanDefinitionsCachingTestControllerModule extends ControllerModule { // Access should be synchronised to allow concurrent test runs. - private static final AtomicReference<Set<BeanDefinition>> foundBeanDefinitions + private static final AtomicReference<Set<Class<?>>> matchedAnnotationClasses = new AtomicReference<>(null); private static final AtomicReference<Set<BeanDefinition>> foundNotificationBeanDefinitions @@ -65,9 +65,9 @@ public class InMemoryDefaultTestModule extends AbstractModule { } @Override - protected Set<BeanDefinition> bindByAnnotation(Set<BeanDefinition> beanDefinitions) { - Set<BeanDefinition> newBeanDefinitions = super.bindByAnnotation(foundBeanDefinitions.get()); - foundBeanDefinitions.compareAndSet(null, Collections.unmodifiableSet(newBeanDefinitions)); + protected Set<Class<?>> bindByAnnotation(Set<Class<?>> matchedClasses) { + Set<Class<?>> newMatchedClasses = super.bindByAnnotation(matchedAnnotationClasses.get()); + matchedAnnotationClasses.compareAndSet(null, Collections.unmodifiableSet(newMatchedClasses)); return null; }
