Repository: ambari Updated Branches: refs/heads/trunk 32f1b05fd -> 477899fb4
AMBARI-9123 - Views: Throwing exception to ambari-server.log on bad view validation (tbeerbower) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/477899fb Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/477899fb Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/477899fb Branch: refs/heads/trunk Commit: 477899fb45d64dc62a01e061982498962a978f70 Parents: 32f1b05 Author: tbeerbower <[email protected]> Authored: Wed Jan 14 11:39:08 2015 -0500 Committer: tbeerbower <[email protected]> Committed: Wed Jan 14 13:06:36 2015 -0500 ---------------------------------------------------------------------- .../internal/ViewInstanceResourceProvider.java | 7 ++ .../server/orm/entities/ViewInstanceEntity.java | 7 +- .../ambari/server/view/ViewContextImpl.java | 3 + .../apache/ambari/server/view/ViewRegistry.java | 11 +-- .../view/validation/ValidationException.java | 34 ++++++++ .../orm/entities/ViewInstanceEntityTest.java | 5 +- .../ambari/server/view/ViewRegistryTest.java | 91 +++++++++++++++++++- .../org/apache/ambari/view/ViewContext.java | 3 +- 8 files changed, 146 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/477899fb/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 08d55f9..28e5e12 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 @@ -36,6 +36,7 @@ import org.apache.ambari.server.orm.entities.ViewInstanceDataEntity; import org.apache.ambari.server.orm.entities.ViewInstanceEntity; import org.apache.ambari.server.view.ViewRegistry; import org.apache.ambari.server.view.validation.InstanceValidationResultImpl; +import org.apache.ambari.server.view.validation.ValidationException; import org.apache.ambari.server.view.validation.ValidationResultImpl; import org.apache.ambari.view.validation.Validator; @@ -384,6 +385,9 @@ public class ViewInstanceResourceProvider extends AbstractResourceProvider { 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); } return null; } @@ -405,6 +409,9 @@ public class ViewInstanceResourceProvider extends AbstractResourceProvider { ViewRegistry.getInstance().updateViewInstance(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); } } return null; http://git-wip-us.apache.org/repos/asf/ambari/blob/477899fb/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java index 54a96ea..d55f949 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java @@ -49,6 +49,7 @@ import org.apache.ambari.server.security.SecurityHelperImpl; import org.apache.ambari.server.security.authorization.AmbariAuthorizationFilter; import org.apache.ambari.server.view.configuration.InstanceConfig; import org.apache.ambari.server.view.validation.InstanceValidationResultImpl; +import org.apache.ambari.server.view.validation.ValidationException; import org.apache.ambari.server.view.validation.ValidationResultImpl; import org.apache.ambari.view.validation.Validator; import org.apache.ambari.view.ResourceProvider; @@ -724,12 +725,12 @@ public class ViewInstanceEntity implements ViewInstanceDefinition { * @param viewEntity the view entity to which this instance will be bound * @param context the validation context * - * @throws IllegalStateException if the instance is not in a valid state + * @throws ValidationException if the instance can not be validated */ - public void validate(ViewEntity viewEntity, Validator.ValidationContext context) throws IllegalStateException { + public void validate(ViewEntity viewEntity, Validator.ValidationContext context) throws ValidationException { InstanceValidationResultImpl result = getValidationResult(viewEntity, context); if (!result.isValid()) { - throw new IllegalStateException(result.toJson()); + throw new ValidationException(result.toJson()); } } http://git-wip-us.apache.org/repos/asf/ambari/blob/477899fb/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 147beaa..438ee75 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 @@ -29,6 +29,7 @@ import org.apache.ambari.server.view.configuration.ViewConfig; import org.apache.ambari.server.view.events.EventImpl; import org.apache.ambari.server.view.persistence.DataStoreImpl; import org.apache.ambari.server.view.persistence.DataStoreModule; +import org.apache.ambari.server.view.validation.ValidationException; import org.apache.ambari.view.AmbariStreamProvider; import org.apache.ambari.view.DataStore; import org.apache.ambari.view.ImpersonatorSetting; @@ -216,6 +217,8 @@ public class ViewContextImpl implements ViewContext, ViewController { String msg = "Caught exception updating the view instance."; LOG.error(msg, e); throw new IllegalStateException(msg, e); + } catch (ValidationException e) { + throw new IllegalArgumentException(e.getMessage()); } } } http://git-wip-us.apache.org/repos/asf/ambari/blob/477899fb/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 2cbaed2..05f9ce1 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 @@ -65,6 +65,7 @@ import org.apache.ambari.server.view.configuration.PersistenceConfig; import org.apache.ambari.server.view.configuration.PropertyConfig; import org.apache.ambari.server.view.configuration.ResourceConfig; import org.apache.ambari.server.view.configuration.ViewConfig; +import org.apache.ambari.server.view.validation.ValidationException; import org.apache.ambari.view.validation.Validator; import org.apache.ambari.view.Masker; import org.apache.ambari.view.SystemException; @@ -458,14 +459,14 @@ public class ViewRegistry { * * @param instanceEntity the view instance entity * - * @throws IllegalStateException if the given instance is not in a valid state + * @throws ValidationException if the given instance fails the validation checks * @throws IllegalArgumentException if the view associated with the given instance * does not exist * @throws SystemException if the instance can not be installed */ @Transactional public void installViewInstance(ViewInstanceEntity instanceEntity) - throws IllegalStateException, IllegalArgumentException, SystemException { + throws ValidationException, IllegalArgumentException, SystemException { ViewEntity viewEntity = getDefinition(instanceEntity.getViewName()); if (viewEntity != null) { @@ -525,11 +526,11 @@ public class ViewRegistry { * * @param instanceEntity the view instance entity * - * @throws IllegalStateException if the given instance is not in a valid state + * @throws ValidationException if the given instance fails the validation checks * @throws SystemException if the instance can not be updated */ public void updateViewInstance(ViewInstanceEntity instanceEntity) - throws IllegalStateException, SystemException { + throws ValidationException, SystemException { ViewEntity viewEntity = getDefinition(instanceEntity.getViewName()); if (viewEntity != null) { @@ -944,7 +945,7 @@ public class ViewRegistry { // create a new view instance definition protected ViewInstanceEntity createViewInstanceDefinition(ViewConfig viewConfig, ViewEntity viewDefinition, InstanceConfig instanceConfig) - throws ClassNotFoundException, SystemException { + throws ValidationException, ClassNotFoundException, SystemException { ViewInstanceEntity viewInstanceDefinition = new ViewInstanceEntity(viewDefinition, instanceConfig); http://git-wip-us.apache.org/repos/asf/ambari/blob/477899fb/ambari-server/src/main/java/org/apache/ambari/server/view/validation/ValidationException.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/view/validation/ValidationException.java b/ambari-server/src/main/java/org/apache/ambari/server/view/validation/ValidationException.java new file mode 100644 index 0000000..6191b24 --- /dev/null +++ b/ambari-server/src/main/java/org/apache/ambari/server/view/validation/ValidationException.java @@ -0,0 +1,34 @@ +/** + * 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. + */ + +package org.apache.ambari.server.view.validation; + +/** + * Internal exception used for view validation failures. + */ +public class ValidationException extends Exception { + + /** + * Constructor. + * + * @param message the exception message + */ + public ValidationException(String message) { + super(message); + } +} http://git-wip-us.apache.org/repos/asf/ambari/blob/477899fb/ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java b/ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java index 3aa17d1..036fa03 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java @@ -28,6 +28,7 @@ import org.apache.ambari.server.view.configuration.InstanceConfigTest; import org.apache.ambari.server.view.configuration.ViewConfig; import org.apache.ambari.server.view.configuration.ViewConfigTest; import org.apache.ambari.server.view.validation.InstanceValidationResultImpl; +import org.apache.ambari.server.view.validation.ValidationException; import org.apache.ambari.server.view.validation.ValidationResultImpl; import org.apache.ambari.view.ResourceProvider; import org.apache.ambari.view.validation.ValidationResult; @@ -446,7 +447,7 @@ public class ViewInstanceEntityTest { try { viewInstanceEntity.validate(viewEntity, Validator.ValidationContext.PRE_CREATE); Assert.fail("Expected an IllegalStateException"); - } catch (IllegalStateException e) { + } catch (ValidationException e) { // expected } } @@ -470,7 +471,7 @@ public class ViewInstanceEntityTest { try { viewInstanceEntity.validate(viewEntity, Validator.ValidationContext.PRE_CREATE); Assert.fail("Expected an IllegalStateException"); - } catch (IllegalStateException e) { + } catch (ValidationException e) { // expected } } http://git-wip-us.apache.org/repos/asf/ambari/blob/477899fb/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 f25bd4d..203ea18 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 @@ -82,6 +82,7 @@ import org.apache.ambari.server.view.configuration.ViewConfig; import org.apache.ambari.server.view.configuration.ViewConfigTest; import org.apache.ambari.server.view.events.EventImpl; import org.apache.ambari.server.view.events.EventImplTest; +import org.apache.ambari.server.view.validation.ValidationException; import org.apache.ambari.view.ViewDefinition; import org.apache.ambari.view.events.Event; import org.apache.ambari.view.events.Listener; @@ -785,13 +786,95 @@ public class ViewRegistryTest { try { registry.installViewInstance(viewInstanceEntity); Assert.fail("expected an IllegalStateException"); - } catch (IllegalStateException e) { + } catch (ValidationException e) { // expected } verify(viewDAO, viewInstanceDAO, securityHelper, resourceTypeDAO); } @Test + public void testInstallViewInstance_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)); + + expect(viewInstanceDAO.merge(viewInstanceEntity)).andReturn(null); + expect(viewInstanceDAO.findByName("MY_VIEW{1.0.0}", viewInstanceEntity.getInstanceName())).andReturn(viewInstanceEntity); + + handlerList.addViewInstance(viewInstanceEntity); + + expect(validator.validateInstance(viewInstanceEntity, Validator.ValidationContext.PRE_CREATE)).andReturn(result).anyTimes(); + expect(result.isValid()).andReturn(true).anyTimes(); + + replay(viewDAO, viewInstanceDAO, securityHelper, handlerList, validator, result); + + registry.addDefinition(viewEntity); + registry.installViewInstance(viewInstanceEntity); + + 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, handlerList, validator, result); + } + + @Test + public void testInstallViewInstance_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)); + + expect(validator.validateInstance(viewInstanceEntity, Validator.ValidationContext.PRE_CREATE)).andReturn(result).anyTimes(); + expect(result.isValid()).andReturn(false).anyTimes(); + + replay(viewDAO, viewInstanceDAO, securityHelper, handlerList, validator, result); + + registry.addDefinition(viewEntity); + + try { + registry.installViewInstance(viewInstanceEntity); + Assert.fail("expected a ValidationException"); + } catch (ValidationException e) { + // expected + } + + Collection<ViewInstanceEntity> viewInstanceDefinitions = registry.getInstanceDefinitions(viewEntity); + + Assert.assertTrue(viewInstanceDefinitions.isEmpty()); + + verify(viewDAO, viewInstanceDAO, securityHelper, handlerList, validator, result); + } + + @Test public void testInstallViewInstance_unknownView() throws Exception { ViewRegistry registry = ViewRegistry.getInstance(); @@ -952,7 +1035,7 @@ public class ViewRegistryTest { try { registry.updateViewInstance(updateInstance); Assert.fail("expected an IllegalStateException"); - } catch (IllegalStateException e) { + } catch (ValidationException e) { // expected } verify(viewDAO, viewInstanceDAO, securityHelper); @@ -1032,8 +1115,8 @@ public class ViewRegistryTest { try { registry.updateViewInstance(updateInstance); - Assert.fail("expected an IllegalStateException"); - } catch (IllegalStateException e) { + Assert.fail("expected a ValidationException"); + } catch (ValidationException e) { // expected } http://git-wip-us.apache.org/repos/asf/ambari/blob/477899fb/ambari-views/src/main/java/org/apache/ambari/view/ViewContext.java ---------------------------------------------------------------------- diff --git a/ambari-views/src/main/java/org/apache/ambari/view/ViewContext.java b/ambari-views/src/main/java/org/apache/ambari/view/ViewContext.java index 1897a0c..432babb 100644 --- a/ambari-views/src/main/java/org/apache/ambari/view/ViewContext.java +++ b/ambari-views/src/main/java/org/apache/ambari/view/ViewContext.java @@ -93,7 +93,8 @@ public interface ViewContext { * @param key the key * @param value the value * - * @throws IllegalStateException if no instance is associated + * @throws IllegalStateException if no instance is associated + * @throws IllegalArgumentException if updating the view instance fails validation */ public void putInstanceData(String key, String value);
