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 <[email protected]>
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
