AMBARI-22110. Some ResourceProviders Are Not Transactional (ncole)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/11163157 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/11163157 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/11163157 Branch: refs/heads/branch-feature-AMBARI-20859 Commit: 11163157134c5e805923a1677c1489783bff0419 Parents: 6d51e01 Author: Nate Cole <[email protected]> Authored: Mon Oct 2 16:43:21 2017 -0400 Committer: Nate Cole <[email protected]> Committed: Mon Oct 2 16:43:21 2017 -0400 ---------------------------------------------------------------------- .../server/controller/ControllerModule.java | 4 + .../controller/ResourceProviderFactory.java | 8 + .../AbstractControllerResourceProvider.java | 4 + .../internal/AlertTargetResourceProvider.java | 3 +- .../internal/ClientConfigResourceProvider.java | 2 - .../internal/DefaultProviderModule.java | 4 - .../internal/ViewInstanceResourceProvider.java | 147 +++++++++++-------- 7 files changed, 101 insertions(+), 71 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/11163157/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 968e9b6..dc97871 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 @@ -62,6 +62,7 @@ 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; +import org.apache.ambari.server.controller.internal.AlertTargetResourceProvider; import org.apache.ambari.server.controller.internal.ClusterStackVersionResourceProvider; import org.apache.ambari.server.controller.internal.ComponentResourceProvider; import org.apache.ambari.server.controller.internal.CredentialResourceProvider; @@ -73,6 +74,7 @@ import org.apache.ambari.server.controller.internal.MemberResourceProvider; import org.apache.ambari.server.controller.internal.RepositoryVersionResourceProvider; import org.apache.ambari.server.controller.internal.ServiceResourceProvider; import org.apache.ambari.server.controller.internal.UpgradeResourceProvider; +import org.apache.ambari.server.controller.internal.ViewInstanceResourceProvider; import org.apache.ambari.server.controller.logging.LoggingRequestHelperFactory; import org.apache.ambari.server.controller.logging.LoggingRequestHelperFactoryImpl; import org.apache.ambari.server.controller.metrics.MetricPropertyProviderFactory; @@ -469,6 +471,8 @@ public class ControllerModule extends AbstractModule { .implement(ResourceProvider.class, Names.named("kerberosDescriptor"), KerberosDescriptorResourceProvider.class) .implement(ResourceProvider.class, Names.named("upgrade"), UpgradeResourceProvider.class) .implement(ResourceProvider.class, Names.named("clusterStackVersion"), ClusterStackVersionResourceProvider.class) + .implement(ResourceProvider.class, Names.named("alertTarget"), AlertTargetResourceProvider.class) + .implement(ResourceProvider.class, Names.named("viewInstance"), ViewInstanceResourceProvider.class) .build(ResourceProviderFactory.class)); install(new FactoryModuleBuilder().implement( http://git-wip-us.apache.org/repos/asf/ambari/blob/11163157/ambari-server/src/main/java/org/apache/ambari/server/controller/ResourceProviderFactory.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/ResourceProviderFactory.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/ResourceProviderFactory.java index 9cd1d74..a198775 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/ResourceProviderFactory.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/ResourceProviderFactory.java @@ -22,8 +22,10 @@ package org.apache.ambari.server.controller; import java.util.Map; import java.util.Set; +import org.apache.ambari.server.controller.internal.AlertTargetResourceProvider; import org.apache.ambari.server.controller.internal.ClusterStackVersionResourceProvider; import org.apache.ambari.server.controller.internal.UpgradeResourceProvider; +import org.apache.ambari.server.controller.internal.ViewInstanceResourceProvider; import org.apache.ambari.server.controller.spi.Resource; import org.apache.ambari.server.controller.spi.Resource.Type; import org.apache.ambari.server.controller.spi.ResourceProvider; @@ -72,4 +74,10 @@ public interface ResourceProviderFactory { @Named("clusterStackVersion") ClusterStackVersionResourceProvider getClusterStackVersionResourceProvider(AmbariManagementController managementController); + @Named("alertTarget") + AlertTargetResourceProvider getAlertTargetResourceProvider(); + + @Named("viewInstance") + ViewInstanceResourceProvider getViewInstanceResourceProvider(); + } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/ambari/blob/11163157/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java index b4b13eb..a98ad46 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java @@ -254,6 +254,10 @@ public abstract class AbstractControllerResourceProvider extends AbstractAuthori return new ClusterKerberosDescriptorResourceProvider(managementController); case LoggingQuery: return new LoggingResourceProvider(propertyIds, keyPropertyIds, managementController); + case AlertTarget: + return resourceProviderFactory.getAlertTargetResourceProvider(); + case ViewInstance: + return resourceProviderFactory.getViewInstanceResourceProvider(); default: throw new IllegalArgumentException("Unknown type " + type); } http://git-wip-us.apache.org/repos/asf/ambari/blob/11163157/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java index ceb767d..43ee7fe 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java @@ -79,7 +79,7 @@ public class AlertTargetResourceProvider extends public static final String ALERT_TARGET_ENABLED = "AlertTarget/enabled"; private static final Set<String> PK_PROPERTY_IDS = new HashSet<>( - Arrays.asList(ALERT_TARGET_ID, ALERT_TARGET_NAME)); + Arrays.asList(ALERT_TARGET_ID, ALERT_TARGET_NAME)); /** * The property ids for an alert target resource. @@ -124,6 +124,7 @@ public class AlertTargetResourceProvider extends /** * Constructor. */ + @Inject AlertTargetResourceProvider() { super(PROPERTY_IDS, KEY_PROPERTY_IDS); http://git-wip-us.apache.org/repos/asf/ambari/blob/11163157/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java index 9f4a4a0..a2a49d7 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java @@ -105,7 +105,6 @@ import org.slf4j.LoggerFactory; import com.google.gson.Gson; import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.AssistedInject; -import com.google.inject.persist.Transactional; /** * Resource provider for client config resources. @@ -162,7 +161,6 @@ public class ClientConfigResourceProvider extends AbstractControllerResourceProv } @Override - @Transactional public Set<Resource> getResources(Request request, Predicate predicate) throws SystemException, UnsupportedPropertyException, NoSuchResourceException, NoSuchParentResourceException { http://git-wip-us.apache.org/repos/asf/ambari/blob/11163157/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java index 5e5bff5..43779a3 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java @@ -66,8 +66,6 @@ public class DefaultProviderModule extends AbstractProviderModule { return new ViewResourceProvider(); case ViewVersion: return new ViewVersionResourceProvider(); - case ViewInstance: - return new ViewInstanceResourceProvider(); case ViewURL: return new ViewURLResourceProvider(); case StackServiceComponentDependency: @@ -94,8 +92,6 @@ public class DefaultProviderModule extends AbstractProviderModule { return new AlertDefinitionResourceProvider(managementController); case AlertHistory: return new AlertHistoryResourceProvider(managementController); - case AlertTarget: - return new AlertTargetResourceProvider(); case AlertGroup: return new AlertGroupResourceProvider(managementController); case AlertNotice: http://git-wip-us.apache.org/repos/asf/ambari/blob/11163157/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java index 88c2a68..9562782 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java @@ -50,6 +50,7 @@ import org.apache.ambari.server.view.validation.ValidationResultImpl; import org.apache.ambari.view.ClusterType; import org.apache.ambari.view.validation.Validator; +import com.google.inject.Inject; import com.google.inject.persist.Transactional; /** @@ -127,6 +128,7 @@ public class ViewInstanceResourceProvider extends AbstractAuthorizedResourceProv /** * Construct a view instance resource provider. */ + @Inject public ViewInstanceResourceProvider() { super(propertyIds, keyPropertyIds); @@ -397,37 +399,9 @@ public class ViewInstanceResourceProvider extends AbstractAuthorizedResourceProv // Create a create command with all properties set. private Command<Void> getCreateCommand(final Map<String, Object> properties) { return new Command<Void>() { - @Transactional @Override public Void invoke() throws AmbariException { - try { - ViewRegistry viewRegistry = ViewRegistry.getInstance(); - ViewInstanceEntity instanceEntity = toEntity(properties, false); - - ViewEntity viewEntity = instanceEntity.getViewEntity(); - String viewName = viewEntity.getCommonName(); - String version = viewEntity.getVersion(); - ViewEntity view = viewRegistry.getDefinition(viewName, version); - - if ( view == null ) { - throw new IllegalStateException("The view " + viewName + " is not registered."); - } - - // the view must be in the DEPLOYED state to create an instance - if (!view.isDeployed()) { - throw new IllegalStateException("The view " + viewName + " is not loaded."); - } - - if (viewRegistry.instanceExists(instanceEntity)) { - throw new DuplicateResourceException("The instance " + instanceEntity.getName() + " already exists."); - } - viewRegistry.installViewInstance(instanceEntity); - } catch (org.apache.ambari.view.SystemException e) { - throw new AmbariException("Caught exception trying to create view instance.", e); - } catch (ValidationException e) { - // results in a BAD_REQUEST (400) response for the validation failure. - throw new IllegalArgumentException(e.getMessage(), e); - } + create(properties); return null; } }; @@ -436,23 +410,9 @@ public class ViewInstanceResourceProvider extends AbstractAuthorizedResourceProv // Create an update command with all properties set. private Command<Void> getUpdateCommand(final Map<String, Object> properties) { return new Command<Void>() { - @Transactional @Override public Void invoke() throws AmbariException { - ViewInstanceEntity instance = toEntity(properties, true); - ViewEntity view = instance.getViewEntity(); - - if (includeInstance(view.getCommonName(), view.getVersion(), instance.getInstanceName(), false)) { - try { - ViewRegistry.getInstance().updateViewInstance(instance); - ViewRegistry.getInstance().updateView(instance); - } catch (org.apache.ambari.view.SystemException e) { - throw new AmbariException("Caught exception trying to update view instance.", e); - } catch (ValidationException e) { - // results in a BAD_REQUEST (400) response for the validation failure. - throw new IllegalArgumentException(e.getMessage(), e); - } - } + update(properties); return null; } }; @@ -463,32 +423,91 @@ public class ViewInstanceResourceProvider extends AbstractAuthorizedResourceProv return new Command<Void>() { @Override public Void invoke() throws AmbariException { - Set<String> requestedIds = getRequestPropertyIds(PropertyHelper.getReadRequest(), predicate); - ViewRegistry viewRegistry = ViewRegistry.getInstance(); - - Set<ViewInstanceEntity> viewInstanceEntities = new HashSet<>(); - - for (ViewEntity viewEntity : viewRegistry.getDefinitions()){ - // the view must be in the DEPLOYED state to delete an instance - if (viewEntity.isDeployed()) { - for (ViewInstanceEntity viewInstanceEntity : viewRegistry.getInstanceDefinitions(viewEntity)){ - Resource resource = toResource(viewInstanceEntity, requestedIds); - if (predicate == null || predicate.evaluate(resource)) { - if (includeInstance(viewInstanceEntity, false)) { - viewInstanceEntities.add(viewInstanceEntity); - } - } + delete(predicate); + return null; + } + }; + } + + @Transactional + void create(Map<String, Object> properties) throws AmbariException { + try { + ViewRegistry viewRegistry = ViewRegistry.getInstance(); + ViewInstanceEntity instanceEntity = toEntity(properties, false); + + ViewEntity viewEntity = instanceEntity.getViewEntity(); + String viewName = viewEntity.getCommonName(); + String version = viewEntity.getVersion(); + ViewEntity view = viewRegistry.getDefinition(viewName, version); + + if ( view == null ) { + throw new IllegalStateException("The view " + viewName + " is not registered."); + } + + // the view must be in the DEPLOYED state to create an instance + if (!view.isDeployed()) { + throw new IllegalStateException("The view " + viewName + " is not loaded."); + } + + if (viewRegistry.instanceExists(instanceEntity)) { + throw new DuplicateResourceException("The instance " + instanceEntity.getName() + " already exists."); + } + viewRegistry.installViewInstance(instanceEntity); + } catch (org.apache.ambari.view.SystemException e) { + throw new AmbariException("Caught exception trying to create view instance.", e); + } catch (ValidationException e) { + // results in a BAD_REQUEST (400) response for the validation failure. + throw new IllegalArgumentException(e.getMessage(), e); + } + } + + @Transactional + void update(Map<String, Object> properties) throws AmbariException { + ViewInstanceEntity instance = toEntity(properties, true); + ViewEntity view = instance.getViewEntity(); + + if (includeInstance(view.getCommonName(), view.getVersion(), instance.getInstanceName(), false)) { + try { + ViewRegistry.getInstance().updateViewInstance(instance); + ViewRegistry.getInstance().updateView(instance); + } catch (org.apache.ambari.view.SystemException e) { + throw new AmbariException("Caught exception trying to update view instance.", e); + } catch (ValidationException e) { + // results in a BAD_REQUEST (400) response for the validation failure. + throw new IllegalArgumentException(e.getMessage(), e); + } + } + + } + + @Transactional + void delete(Predicate predicate) { + Set<String> requestedIds = getRequestPropertyIds(PropertyHelper.getReadRequest(), predicate); + ViewRegistry viewRegistry = ViewRegistry.getInstance(); + + Set<ViewInstanceEntity> viewInstanceEntities = new HashSet<>(); + + for (ViewEntity viewEntity : viewRegistry.getDefinitions()){ + // the view must be in the DEPLOYED state to delete an instance + if (viewEntity.isDeployed()) { + for (ViewInstanceEntity viewInstanceEntity : viewRegistry.getInstanceDefinitions(viewEntity)){ + Resource resource = toResource(viewInstanceEntity, requestedIds); + if (predicate == null || predicate.evaluate(resource)) { + if (includeInstance(viewInstanceEntity, false)) { + viewInstanceEntities.add(viewInstanceEntity); } } } - for (ViewInstanceEntity viewInstanceEntity : viewInstanceEntities) { - viewRegistry.uninstallViewInstance(viewInstanceEntity); - } - return null; } - }; + } + for (ViewInstanceEntity viewInstanceEntity : viewInstanceEntities) { + viewRegistry.uninstallViewInstance(viewInstanceEntity); + } } + + + // get the icon path private static String getIconPath(String contextPath, String iconPath){ return iconPath == null || iconPath.length() == 0 ? null :
