Repository: ambari Updated Branches: refs/heads/trunk 7de7343bc -> 1a4d5aa6d
AMBARI-9112 - Views: validation for properties is ignored in PUT requests (tbeerbower) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/1a4d5aa6 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/1a4d5aa6 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/1a4d5aa6 Branch: refs/heads/trunk Commit: 1a4d5aa6d0f1e59f0387ab7d3986964c15c45cdd Parents: 7de7343 Author: tbeerbower <[email protected]> Authored: Tue Jan 13 15:52:40 2015 -0500 Committer: tbeerbower <[email protected]> Committed: Wed Jan 14 11:00:55 2015 -0500 ---------------------------------------------------------------------- .../internal/ViewInstanceResourceProvider.java | 5 +- .../ambari/server/view/ViewContextImpl.java | 23 +++-- .../apache/ambari/server/view/ViewRegistry.java | 56 ++++++++--- .../ambari/server/view/ViewRegistryTest.java | 97 +++++++++++++++++++- 4 files changed, 159 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/1a4d5aa6/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 12a8bee..08d55f9 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 @@ -18,6 +18,7 @@ package org.apache.ambari.server.controller.internal; +import com.google.inject.persist.Transactional; import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.DuplicateResourceException; import org.apache.ambari.server.controller.spi.NoSuchParentResourceException; @@ -296,7 +297,7 @@ public class ViewInstanceResourceProvider extends AbstractResourceProvider { ViewInstanceEntity viewInstanceEntity = null; if (update) { - viewInstanceEntity = viewRegistry.getInstanceDefinition(commonViewName, version, name); + viewInstanceEntity = viewRegistry.getViewInstanceEntity(viewName, name); } if (viewInstanceEntity == null) { @@ -356,6 +357,7 @@ public class ViewInstanceResourceProvider extends AbstractResourceProvider { // 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 { @@ -391,6 +393,7 @@ public class ViewInstanceResourceProvider extends AbstractResourceProvider { // 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 { http://git-wip-us.apache.org/repos/asf/ambari/blob/1a4d5aa6/ambari-server/src/main/java/org/apache/ambari/server/view/ViewContextImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/view/ViewContextImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/view/ViewContextImpl.java index dd7417a..147beaa 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/view/ViewContextImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/view/ViewContextImpl.java @@ -20,6 +20,7 @@ package org.apache.ambari.server.view; import com.google.inject.Guice; import com.google.inject.Injector; +import com.google.inject.persist.Transactional; import org.apache.ambari.server.orm.entities.PermissionEntity; import org.apache.ambari.server.orm.entities.ViewEntity; import org.apache.ambari.server.orm.entities.ViewInstanceEntity; @@ -198,16 +199,24 @@ public class ViewContextImpl implements ViewContext, ViewController { } } + @Transactional @Override public void putInstanceData(String key, String value) { checkInstance(); - viewInstanceEntity.putInstanceData(key, value); - try { - viewRegistry.updateViewInstance(viewInstanceEntity); - } catch (SystemException e) { - String msg = "Caught exception updating the view instance."; - LOG.error(msg, e); - throw new IllegalStateException(msg, e); + + ViewInstanceEntity updateInstance = + viewRegistry.getViewInstanceEntity(viewInstanceEntity.getViewName(), viewInstanceEntity.getInstanceName()); + + if (updateInstance != null) { + updateInstance.putInstanceData(key, value); + + try { + viewRegistry.updateViewInstance(updateInstance); + } catch (SystemException e) { + String msg = "Caught exception updating the view instance."; + LOG.error(msg, e); + throw new IllegalStateException(msg, e); + } } } http://git-wip-us.apache.org/repos/asf/ambari/blob/1a4d5aa6/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java b/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java index 57a05f1..2cbaed2 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java @@ -333,14 +333,14 @@ public class ViewRegistry { } /** - * Get the instance definition for the given view name and instance name. - * - * @param viewName the view name - * @param version the version - * @param instanceName the instance name - * - * @return the view instance definition for the given view and instance name - */ + * Get the instance definition for the given view name and instance name. + * + * @param viewName the view name + * @param version the version + * @param instanceName the instance name + * + * @return the view instance definition for the given view and instance name + */ public ViewInstanceEntity getInstanceDefinition(String viewName, String version, String instanceName) { Map<String, ViewInstanceEntity> viewInstanceDefinitionMap = viewInstanceDefinitions.get(getDefinition(viewName, version)); @@ -535,10 +535,24 @@ public class ViewRegistry { if (viewEntity != null) { instanceEntity.validate(viewEntity, Validator.ValidationContext.PRE_UPDATE); instanceDAO.merge(instanceEntity); + + syncViewInstance(instanceEntity); } } /** + * Get a view instance entity for the given view name and instance name. + * + * @param viewName the view name + * @param instanceName the instance name + * + * @return a view instance entity for the given view name and instance name. + */ + public ViewInstanceEntity getViewInstanceEntity(String viewName, String instanceName) { + return instanceDAO.findByName(viewName, instanceName); + } + + /** * Uninstall a view instance for the view with the given view name. * * @param instanceEntity the view instance entity @@ -1152,14 +1166,32 @@ public class ViewRegistry { instanceDefinitions.add(persistedInstance); } } else { - instance.setResource(persistedInstance.getResource()); - instance.setViewInstanceId(persistedInstance.getViewInstanceId()); - instance.setData(persistedInstance.getData()); - instance.setEntities(persistedInstance.getEntities()); + syncViewInstance(instance, persistedInstance); } } } + // sync the given view instance entity to the matching view instance entity in the registry + private void syncViewInstance(ViewInstanceEntity instanceEntity) { + String viewName = instanceEntity.getViewDefinition().getViewName(); + String version = instanceEntity.getViewDefinition().getVersion(); + String instanceName = instanceEntity.getInstanceName(); + + ViewInstanceEntity registryEntry = getInstanceDefinition(viewName, version, instanceName); + if (registryEntry != null) { + syncViewInstance(registryEntry, instanceEntity); + } + } + + // sync a given view instance entity with another given view instance entity + private void syncViewInstance(ViewInstanceEntity instance1, ViewInstanceEntity instance2) { + instance1.setResource(instance2.getResource()); + instance1.setViewInstanceId(instance2.getViewInstanceId()); + instance1.setData(instance2.getData()); + instance1.setEntities(instance2.getEntities()); + instance1.setProperties(instance2.getProperties()); + } + // create an admin resource to represent a view instance private ResourceEntity createViewInstanceResource(ResourceTypeEntity resourceTypeEntity) { ResourceEntity resourceEntity = new ResourceEntity(); http://git-wip-us.apache.org/repos/asf/ambari/blob/1a4d5aa6/ambari-server/src/test/java/org/apache/ambari/server/view/ViewRegistryTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/view/ViewRegistryTest.java b/ambari-server/src/test/java/org/apache/ambari/server/view/ViewRegistryTest.java index 26a9b36..f25bd4d 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/view/ViewRegistryTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/view/ViewRegistryTest.java @@ -85,6 +85,8 @@ import org.apache.ambari.server.view.events.EventImplTest; import org.apache.ambari.view.ViewDefinition; import org.apache.ambari.view.events.Event; import org.apache.ambari.view.events.Listener; +import org.apache.ambari.view.validation.ValidationResult; +import org.apache.ambari.view.validation.Validator; import org.easymock.Capture; import org.easymock.EasyMock; import org.junit.Assert; @@ -832,7 +834,7 @@ public class ViewRegistryTest { ViewInstanceEntity updateInstance = getViewInstanceEntity(viewEntity, config.getInstances().get(0)); expect(viewInstanceDAO.merge(viewInstanceEntity)).andReturn(viewInstanceEntity); - expect(viewInstanceDAO.findByName("MY_VIEW{1.0.0}", viewInstanceEntity.getInstanceName())).andReturn(viewInstanceEntity); + expect(viewInstanceDAO.findByName("MY_VIEW{1.0.0}", viewInstanceEntity.getInstanceName())).andReturn(viewInstanceEntity).anyTimes(); replay(viewDAO, viewInstanceDAO, securityHelper); @@ -940,7 +942,7 @@ public class ViewRegistryTest { ViewInstanceEntity updateInstance = getViewInstanceEntity(viewEntity, invalidConfig.getInstances().get(0)); expect(viewInstanceDAO.merge(viewInstanceEntity)).andReturn(null); - expect(viewInstanceDAO.findByName("MY_VIEW{1.0.0}", viewInstanceEntity.getInstanceName())).andReturn(viewInstanceEntity); + expect(viewInstanceDAO.findByName("MY_VIEW{1.0.0}", viewInstanceEntity.getInstanceName())).andReturn(viewInstanceEntity).anyTimes(); replay(viewDAO, viewInstanceDAO, securityHelper); @@ -957,6 +959,97 @@ public class ViewRegistryTest { } @Test + public void testUpdateViewInstance_validatorPass() throws Exception { + + ViewRegistry registry = ViewRegistry.getInstance(); + + Properties properties = new Properties(); + properties.put("p1", "v1"); + + Configuration ambariConfig = new Configuration(properties); + Validator validator = createNiceMock(Validator.class); + ValidationResult result = createNiceMock(ValidationResult.class); + + ViewConfig config = ViewConfigTest.getConfig(xml_valid_instance); + ViewEntity viewEntity = getViewEntity(config, ambariConfig, getClass().getClassLoader(), ""); + viewEntity.setValidator(validator); + ViewInstanceEntity viewInstanceEntity = getViewInstanceEntity(viewEntity, config.getInstances().get(0)); + ViewInstanceEntity updateInstance = getViewInstanceEntity(viewEntity, config.getInstances().get(0)); + + expect(viewInstanceDAO.merge(viewInstanceEntity)).andReturn(viewInstanceEntity); + expect(viewInstanceDAO.findByName("MY_VIEW{1.0.0}", viewInstanceEntity.getInstanceName())).andReturn(viewInstanceEntity).anyTimes(); + + expect(validator.validateInstance(viewInstanceEntity, Validator.ValidationContext.PRE_UPDATE)).andReturn(result).anyTimes(); + expect(result.isValid()).andReturn(true).anyTimes(); + + replay(viewDAO, viewInstanceDAO, securityHelper, validator, result); + + registry.addDefinition(viewEntity); + registry.installViewInstance(viewInstanceEntity); + + registry.updateViewInstance(updateInstance); + + Collection<ViewInstanceEntity> viewInstanceDefinitions = registry.getInstanceDefinitions(viewEntity); + + Assert.assertEquals(1, viewInstanceDefinitions.size()); + + ViewInstanceEntity instanceEntity = viewInstanceDefinitions.iterator().next(); + Assert.assertEquals("v2-1", instanceEntity.getProperty("p2").getValue() ); + + Assert.assertEquals(viewInstanceEntity, viewInstanceDefinitions.iterator().next()); + + verify(viewDAO, viewInstanceDAO, securityHelper, validator, result); + } + + @Test + public void testUpdateViewInstance_validatorFail() throws Exception { + + ViewRegistry registry = ViewRegistry.getInstance(); + + Properties properties = new Properties(); + properties.put("p1", "v1"); + + Configuration ambariConfig = new Configuration(properties); + Validator validator = createNiceMock(Validator.class); + ValidationResult result = createNiceMock(ValidationResult.class); + + ViewConfig config = ViewConfigTest.getConfig(xml_valid_instance); + ViewEntity viewEntity = getViewEntity(config, ambariConfig, getClass().getClassLoader(), ""); + viewEntity.setValidator(validator); + ViewInstanceEntity viewInstanceEntity = getViewInstanceEntity(viewEntity, config.getInstances().get(0)); + ViewInstanceEntity updateInstance = getViewInstanceEntity(viewEntity, config.getInstances().get(0)); + + expect(viewInstanceDAO.merge(viewInstanceEntity)).andReturn(viewInstanceEntity); + expect(viewInstanceDAO.findByName("MY_VIEW{1.0.0}", viewInstanceEntity.getInstanceName())).andReturn(viewInstanceEntity).anyTimes(); + + expect(validator.validateInstance(viewInstanceEntity, Validator.ValidationContext.PRE_UPDATE)).andReturn(result).anyTimes(); + expect(result.isValid()).andReturn(false).anyTimes(); + + replay(viewDAO, viewInstanceDAO, securityHelper, validator, result); + + registry.addDefinition(viewEntity); + registry.installViewInstance(viewInstanceEntity); + + try { + registry.updateViewInstance(updateInstance); + Assert.fail("expected an IllegalStateException"); + } catch (IllegalStateException e) { + // expected + } + + Collection<ViewInstanceEntity> viewInstanceDefinitions = registry.getInstanceDefinitions(viewEntity); + + Assert.assertEquals(1, viewInstanceDefinitions.size()); + + ViewInstanceEntity instanceEntity = viewInstanceDefinitions.iterator().next(); + Assert.assertEquals("v2-1", instanceEntity.getProperty("p2").getValue() ); + + Assert.assertEquals(viewInstanceEntity, viewInstanceDefinitions.iterator().next()); + + verify(viewDAO, viewInstanceDAO, securityHelper, validator, result); + } + + @Test public void testRemoveInstanceData() throws Exception { ViewRegistry registry = ViewRegistry.getInstance();
