This is an automated email from the ASF dual-hosted git repository. adoroszlai pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/ambari.git
The following commit(s) were added to refs/heads/trunk by this push: new 7e5cf0a AMBARI-25025. Duplicate kerberos_descriptor name reported as HTTP 500 (#2707) 7e5cf0a is described below commit 7e5cf0afbded4326883d9779b85024f476bfb62a Author: Doroszlai, Attila <6454655+adorosz...@users.noreply.github.com> AuthorDate: Mon Dec 10 21:46:07 2018 +0100 AMBARI-25025. Duplicate kerberos_descriptor name reported as HTTP 500 (#2707) --- .../KerberosDescriptorResourceProvider.java | 108 +++++++----- .../orm/entities/KerberosDescriptorEntity.java | 20 +++ .../KerberosDescriptorResourceProviderTest.java | 190 ++++++++++++--------- 3 files changed, 186 insertions(+), 132 deletions(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/KerberosDescriptorResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/KerberosDescriptorResourceProvider.java index d02d64a..6e20c8a 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/KerberosDescriptorResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/KerberosDescriptorResourceProvider.java @@ -1,3 +1,20 @@ +/* + * 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 + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * 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.controller.internal; import java.util.Collections; @@ -26,32 +43,16 @@ import org.apache.ambari.server.topology.KerberosDescriptorFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Sets; +import com.google.common.collect.ImmutableSet; import com.google.inject.assistedinject.Assisted; -/** - * 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 - * <p/> - * http://www.apache.org/licenses/LICENSE-2.0 - * <p/> - * 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. - */ public class KerberosDescriptorResourceProvider extends AbstractControllerResourceProvider { private static final Logger LOGGER = LoggerFactory.getLogger(KerberosDescriptorResourceProvider.class); - private static final String KERBEROS_DESCRIPTOR_NAME_PROPERTY_ID = + static final String KERBEROS_DESCRIPTOR_NAME_PROPERTY_ID = PropertyHelper.getPropertyId("KerberosDescriptors", "kerberos_descriptor_name"); private static final String KERBEROS_DESCRIPTOR_TEXT_PROPERTY_ID = @@ -60,16 +61,12 @@ public class KerberosDescriptorResourceProvider extends AbstractControllerResour /** * The key property ids for a KerberosDescriptor resource. */ - private static final Map<Resource.Type, String> keyPropertyIds = ImmutableMap.<Resource.Type, String>builder() - .put(Resource.Type.KerberosDescriptor, KERBEROS_DESCRIPTOR_NAME_PROPERTY_ID) - .build(); + private static final Map<Resource.Type, String> KEY_PROPERTY_IDS = ImmutableMap.of(Resource.Type.KerberosDescriptor, KERBEROS_DESCRIPTOR_NAME_PROPERTY_ID); /** * The property ids for a KerberosDescriptor resource. */ - private static final Set<String> propertyIds = Sets.newHashSet( - KERBEROS_DESCRIPTOR_NAME_PROPERTY_ID, - KERBEROS_DESCRIPTOR_TEXT_PROPERTY_ID); + private static final Set<String> PROPERTY_IDS = ImmutableSet.of(KERBEROS_DESCRIPTOR_NAME_PROPERTY_ID, KERBEROS_DESCRIPTOR_TEXT_PROPERTY_ID); private KerberosDescriptorDAO kerberosDescriptorDAO; @@ -80,7 +77,7 @@ public class KerberosDescriptorResourceProvider extends AbstractControllerResour KerberosDescriptorResourceProvider(KerberosDescriptorDAO kerberosDescriptorDAO, KerberosDescriptorFactory kerberosDescriptorFactory, @Assisted AmbariManagementController managementController) { - super(Resource.Type.KerberosDescriptor, propertyIds, keyPropertyIds, managementController); + super(Resource.Type.KerberosDescriptor, PROPERTY_IDS, KEY_PROPERTY_IDS, managementController); this.kerberosDescriptorDAO = kerberosDescriptorDAO; this.kerberosDescriptorFactory = kerberosDescriptorFactory; } @@ -92,22 +89,24 @@ public class KerberosDescriptorResourceProvider extends AbstractControllerResour } @Override - public RequestStatus createResources(Request request) throws SystemException, UnsupportedPropertyException, - ResourceAlreadyExistsException, NoSuchParentResourceException { - + public RequestStatus createResources(Request request) throws ResourceAlreadyExistsException { String name = getNameFromRequest(request); String descriptor = getRawKerberosDescriptorFromRequest(request); + if (kerberosDescriptorDAO.findByName(name) != null) { + String msg = String.format("Kerberos descriptor named %s already exists", name); + LOGGER.info(msg); + throw new ResourceAlreadyExistsException(msg); + } + KerberosDescriptor kerberosDescriptor = kerberosDescriptorFactory.createKerberosDescriptor(name, descriptor); kerberosDescriptorDAO.create(kerberosDescriptor.toEntity()); return getRequestStatus(null); } - @Override - public Set<Resource> getResources(Request request, Predicate predicate) throws SystemException, - UnsupportedPropertyException, NoSuchResourceException, NoSuchParentResourceException { + public Set<Resource> getResources(Request request, Predicate predicate) throws NoSuchResourceException, NoSuchParentResourceException { List<KerberosDescriptorEntity> results = null; boolean applyPredicate = false; @@ -150,7 +149,7 @@ public class KerberosDescriptorResourceProvider extends AbstractControllerResour return resources; } - private void toResource(Resource resource, KerberosDescriptorEntity entity, Set<String> requestPropertyIds) { + private static void toResource(Resource resource, KerberosDescriptorEntity entity, Set<String> requestPropertyIds) { setResourceProperty(resource, KERBEROS_DESCRIPTOR_NAME_PROPERTY_ID, entity.getName(), requestPropertyIds); setResourceProperty(resource, KERBEROS_DESCRIPTOR_TEXT_PROPERTY_ID, entity.getKerberosDescriptorText(), requestPropertyIds); } @@ -179,26 +178,41 @@ public class KerberosDescriptorResourceProvider extends AbstractControllerResour @Override protected Set<String> getPKPropertyIds() { - return Collections.emptySet(); + return ImmutableSet.copyOf(KEY_PROPERTY_IDS.values()); } - private String getRawKerberosDescriptorFromRequest(Request request) throws UnsupportedPropertyException { - if (request.getRequestInfoProperties() == null || - !request.getRequestInfoProperties().containsKey(Request.REQUEST_INFO_BODY_PROPERTY)) { - LOGGER.error("Could not find the raw request body in the request: {}", request); - throw new UnsupportedPropertyException(Resource.Type.KerberosDescriptor, - Collections.singleton(Request.REQUEST_INFO_BODY_PROPERTY)); + /** + * @throws IllegalArgumentException if descriptor text is not found or is empty + */ + private static String getRawKerberosDescriptorFromRequest(Request request) { + Map<String, String> requestInfoProperties = request.getRequestInfoProperties(); + if (requestInfoProperties != null) { + String descriptorText = requestInfoProperties.get(Request.REQUEST_INFO_BODY_PROPERTY); + if (!Strings.isNullOrEmpty(descriptorText)) { + return descriptorText; + } } - return request.getRequestInfoProperties().get(Request.REQUEST_INFO_BODY_PROPERTY); + + String msg = "No Kerberos descriptor found in the request body"; + LOGGER.error(msg); + throw new IllegalArgumentException(msg); } - private String getNameFromRequest(Request request) throws UnsupportedPropertyException { - if (request.getProperties() == null || !request.getProperties().iterator().hasNext()) { - LOGGER.error("There is no {} property id in the request {}", KERBEROS_DESCRIPTOR_NAME_PROPERTY_ID, request); - throw new UnsupportedPropertyException(Resource.Type.KerberosDescriptor, - Collections.singleton(KERBEROS_DESCRIPTOR_NAME_PROPERTY_ID)); + /** + * @throws IllegalArgumentException if name is not found or is empty + */ + private static String getNameFromRequest(Request request) { + if (request.getProperties() != null && !request.getProperties().isEmpty()) { + Map<String, Object> properties = request.getProperties().iterator().next(); + Object name = properties.get(KERBEROS_DESCRIPTOR_NAME_PROPERTY_ID); + if (name != null) { + return String.valueOf(name); + } } - return (String) request.getProperties().iterator().next().get(KERBEROS_DESCRIPTOR_NAME_PROPERTY_ID); + + String msg = "No name provided for the Kerberos descriptor"; + LOGGER.error(msg); + throw new IllegalArgumentException(msg); } } diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/KerberosDescriptorEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/KerberosDescriptorEntity.java index 5fdda87..8d0102c 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/KerberosDescriptorEntity.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/KerberosDescriptorEntity.java @@ -1,5 +1,7 @@ package org.apache.ambari.server.orm.entities; +import java.util.Objects; + import javax.persistence.Column; import javax.persistence.Entity; import javax.persistence.Id; @@ -53,4 +55,22 @@ public class KerberosDescriptorEntity { public void setKerberosDescriptorText(String kerberosDescriptorText) { this.kerberosDescriptorText = kerberosDescriptorText; } + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + KerberosDescriptorEntity other = (KerberosDescriptorEntity) obj; + return Objects.equals(name, other.name) && + Objects.equals(kerberosDescriptorText, other.kerberosDescriptorText); + } + + @Override + public int hashCode() { + return Objects.hash(name, kerberosDescriptorText); + } } diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/KerberosDescriptorResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/KerberosDescriptorResourceProviderTest.java index caea9f2..fcdadc5 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/KerberosDescriptorResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/KerberosDescriptorResourceProviderTest.java @@ -1,86 +1,82 @@ +/* + * 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 + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * 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.controller.internal; -import static org.easymock.EasyMock.anyString; +import static org.apache.ambari.server.controller.internal.KerberosDescriptorResourceProvider.KERBEROS_DESCRIPTOR_NAME_PROPERTY_ID; +import static org.easymock.EasyMock.anyObject; import static org.easymock.EasyMock.capture; +import static org.easymock.EasyMock.eq; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.expectLastCall; +import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.reset; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; import java.util.Map; import java.util.Set; +import javax.persistence.PersistenceException; + import org.apache.ambari.server.controller.spi.Request; -import org.apache.ambari.server.controller.spi.UnsupportedPropertyException; +import org.apache.ambari.server.controller.spi.ResourceAlreadyExistsException; import org.apache.ambari.server.orm.dao.KerberosDescriptorDAO; import org.apache.ambari.server.orm.entities.KerberosDescriptorEntity; import org.apache.ambari.server.topology.KerberosDescriptorFactory; -import org.apache.ambari.server.topology.KerberosDescriptorImpl; import org.easymock.Capture; import org.easymock.EasyMock; import org.easymock.EasyMockRule; import org.easymock.Mock; -import org.easymock.MockType; -import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -/** - * 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 - * <p/> - * http://www.apache.org/licenses/LICENSE-2.0 - * <p/> - * 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. - */ +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; public class KerberosDescriptorResourceProviderTest { - private static final String TEST_KERBEROS_DESCRIPTOR_NAME = "descriptor-name-0"; - private static final String TEST_KERBEROS_DESCRIPTOR = "descriptor"; - public static final String KERBEROS_DESCRIPTORS_KERBEROS_DESCRIPTOR_NAME = "KerberosDescriptors/kerberos_descriptor_name"; - @Rule - public EasyMockRule mocks = new EasyMockRule(this); + public final EasyMockRule mocks = new EasyMockRule(this); - @Mock(type = MockType.STRICT) + @Mock private KerberosDescriptorDAO kerberosDescriptorDAO; - @Mock(type = MockType.STRICT) - private KerberosDescriptorFactory kerberosDescriptorFactory; + private final KerberosDescriptorFactory kerberosDescriptorFactory = new KerberosDescriptorFactory(); - @Mock(type = MockType.STRICT) + @Mock private Request request; private KerberosDescriptorResourceProvider kerberosDescriptorResourceProvider; @Before public void before() { - reset(request); - + reset(request, kerberosDescriptorDAO); + kerberosDescriptorResourceProvider = new KerberosDescriptorResourceProvider(kerberosDescriptorDAO, kerberosDescriptorFactory, null); + expect(kerberosDescriptorDAO.findByName(anyObject())).andStubReturn(null); } - @Test(expected = UnsupportedPropertyException.class) - public void testCreateShouldThrowExceptionWhenNoDescriptorProvided() throws Exception { - + @Test(expected = IllegalArgumentException.class) + public void rejectsCreateWithoutDescriptorText() throws Exception { // GIVEN - EasyMock.expect(request.getProperties()).andReturn(requestPropertySet(KERBEROS_DESCRIPTORS_KERBEROS_DESCRIPTOR_NAME, - TEST_KERBEROS_DESCRIPTOR_NAME)).times(3); - EasyMock.expect(request.getRequestInfoProperties()).andReturn(requestInfoPropertyMap("", "")).times(2); - EasyMock.replay(request); - - kerberosDescriptorResourceProvider = new KerberosDescriptorResourceProvider(kerberosDescriptorDAO, - kerberosDescriptorFactory, null); + expect(request.getProperties()).andReturn(descriptorNamed("any name")).anyTimes(); + expect(request.getRequestInfoProperties()).andReturn(ImmutableMap.of()).anyTimes(); + replay(request); // WHEN kerberosDescriptorResourceProvider.createResources(request); @@ -89,15 +85,11 @@ public class KerberosDescriptorResourceProviderTest { // exception is thrown } - @Test(expected = UnsupportedPropertyException.class) - public void testCreateShouldThrowExceptionWhenNoNameProvided() throws Exception { - + @Test(expected = IllegalArgumentException.class) + public void rejectsCreateWithoutName() throws Exception { // GIVEN - EasyMock.expect(request.getProperties()).andReturn(emptyRequestPropertySet()).times(2); - EasyMock.replay(request); - - kerberosDescriptorResourceProvider = new KerberosDescriptorResourceProvider(kerberosDescriptorDAO, - kerberosDescriptorFactory, null); + expect(request.getProperties()).andReturn(ImmutableSet.of()).anyTimes(); + replay(request); // WHEN kerberosDescriptorResourceProvider.createResources(request); @@ -106,55 +98,83 @@ public class KerberosDescriptorResourceProviderTest { // exception is thrown } - @Test - public void testShoudCreateResourceWhenNameAndDescriptorProvided() throws Exception { + public void acceptsValidRequest() throws Exception { + // GIVEN + String name = "some name", text = "any text"; + Capture<KerberosDescriptorEntity> entityCapture = creatingDescriptor(name, text); + replay(request, kerberosDescriptorDAO); + + // WHEN + kerberosDescriptorResourceProvider.createResources(request); + + // THEN + verifyDescriptorCreated(entityCapture, name, text); + } + + @Test(expected = ResourceAlreadyExistsException.class) + public void rejectsDuplicateName() throws Exception { + String name = "any name"; + descriptorAlreadyExists(name); + tryingToCreateDescriptor(name, "any text"); + replay(request, kerberosDescriptorDAO); + kerberosDescriptorResourceProvider.createResources(request); + } + + @Test + public void canCreateDescriptorWithDifferentName() throws Exception { // GIVEN - kerberosDescriptorResourceProvider = new KerberosDescriptorResourceProvider(kerberosDescriptorDAO, - kerberosDescriptorFactory, null); - - EasyMock.expect(request.getProperties()) - .andReturn(requestPropertySet(KERBEROS_DESCRIPTORS_KERBEROS_DESCRIPTOR_NAME, TEST_KERBEROS_DESCRIPTOR_NAME)) - .times(3); - EasyMock.expect(request.getRequestInfoProperties()) - .andReturn(requestInfoPropertyMap(Request.REQUEST_INFO_BODY_PROPERTY, TEST_KERBEROS_DESCRIPTOR)) - .times(3); - EasyMock.expect(kerberosDescriptorFactory.createKerberosDescriptor(anyString(), anyString())) - .andReturn(new KerberosDescriptorImpl(TEST_KERBEROS_DESCRIPTOR_NAME, TEST_KERBEROS_DESCRIPTOR)); + descriptorAlreadyExists("some name"); - Capture<KerberosDescriptorEntity> entityCapture = EasyMock.newCapture(); - kerberosDescriptorDAO.create(capture(entityCapture)); + String name = "another name", text = "any text"; + Capture<KerberosDescriptorEntity> entityCapture = creatingDescriptor(name, text); - EasyMock.replay(request, kerberosDescriptorFactory, kerberosDescriptorDAO); + replay(request, kerberosDescriptorDAO); // WHEN kerberosDescriptorResourceProvider.createResources(request); // THEN - Assert.assertNotNull(entityCapture.getValue()); - Assert.assertEquals("The resource name is invalid!", TEST_KERBEROS_DESCRIPTOR_NAME, entityCapture.getValue() - .getName()); + verifyDescriptorCreated(entityCapture, name, text); + } + private void verifyDescriptorCreated(Capture<KerberosDescriptorEntity> entityCapture, String name, String text) { + assertNotNull(entityCapture.getValue()); + assertEquals(name, entityCapture.getValue().getName()); + assertEquals(text, entityCapture.getValue().getKerberosDescriptorText()); } - private Set<Map<String, Object>> emptyRequestPropertySet() { - return Collections.emptySet(); + private void descriptorAlreadyExists(String name) { + KerberosDescriptorEntity entity = new KerberosDescriptorEntity(); + entity.setName(name); + expect(kerberosDescriptorDAO.findByName(eq(name))).andReturn(entity).anyTimes(); + + kerberosDescriptorDAO.create(eq(entity)); + expectLastCall().andThrow(new PersistenceException()).anyTimes(); } + private Capture<KerberosDescriptorEntity> creatingDescriptor(String name, String text) { + tryingToCreateDescriptor(name, text); + + Capture<KerberosDescriptorEntity> entityCapture = EasyMock.newCapture(); + kerberosDescriptorDAO.create(capture(entityCapture)); + expectLastCall().anyTimes(); + + return entityCapture; + } + + private void tryingToCreateDescriptor(String name, String text) { + expect(request.getProperties()).andReturn(descriptorNamed(name)).anyTimes(); + expect(request.getRequestInfoProperties()).andReturn(descriptorWithText(text)).anyTimes(); + } - private Map<String, String> requestInfoPropertyMap(String propertyKey, String propertyValue) { - Map<String, String> propsMap = new HashMap<>(); - propsMap.put(propertyKey, propertyValue); - return propsMap; + private static Map<String, String> descriptorWithText(String text) { + return ImmutableMap.of(Request.REQUEST_INFO_BODY_PROPERTY, text); } - private Set<Map<String, Object>> requestPropertySet(String testPropertyKey, String testPropertyValue) { - Set<Map<String, Object>> invalidProps = new HashSet<>(); - Map<String, Object> invalidMap = new HashMap<>(); - invalidMap.put(testPropertyKey, testPropertyValue); - invalidProps.add(invalidMap); - return invalidProps; + private static Set<Map<String, Object>> descriptorNamed(String name) { + return ImmutableSet.of(ImmutableMap.of(KERBEROS_DESCRIPTOR_NAME_PROPERTY_ID, name)); } } \ No newline at end of file