Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/927#discussion_r160116429
--- Diff:
rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/BundleAndTypeResourcesTest.java
---
@@ -1095,4 +1110,268 @@ private boolean checkTraits(boolean
currentExpectedToBeWorking) {
return currentExpectedToBeWorking;
}
+ @Test
+ public void testAddSameTypeTwiceInSameBundle_SilentlyDeduped() throws
Exception {
+ final String symbolicName =
"test.duplicate.type."+JavaClassNames.niceClassAndMethod();
+ final String entityName = symbolicName+".type";
+
+ File jar = createJar(ImmutableMap.<String,
String>of("catalog.bom", Joiner.on("\n").join(
+ "brooklyn.catalog:",
+ " bundle: " + symbolicName,
+ " version: " + TEST_VERSION,
+ " items:",
+ " - id: " + entityName,
+ " itemType: entity",
+ " name: T",
+ " item:",
+ " type:
org.apache.brooklyn.core.test.entity.TestEntity",
+ " - id: " + entityName,
+ " itemType: entity",
+ " name: T",
+ " item:",
+ " type:
org.apache.brooklyn.core.test.entity.TestEntity")));
+
+ Response result = client().path("/catalog/bundles")
+ .header(HttpHeaders.CONTENT_TYPE, "application/x-jar")
+ .post(Streams.readFully(new FileInputStream(jar)));
+ HttpAsserts.assertHealthyStatusCode(result.getStatus());
+
+ TypeSummary entity = client().path("/catalog/types/" + entityName
+ "/" + TEST_VERSION)
+ .get(TypeSummary.class);
+ assertEquals(entity.getDisplayName(), "T");
+
+ List<TypeSummary> entities = client().path("/catalog/types/" +
entityName)
+ .get(new GenericType<List<TypeSummary>>() {});
+ Asserts.assertSize(entities, 1);
+ assertEquals(Iterables.getOnlyElement(entities), entity);
+
+ BundleSummary bundle = client().path("/catalog/bundles/" +
symbolicName + "/" + TEST_VERSION)
+ .get(BundleSummary.class);
+ Asserts.assertSize(bundle.getTypes(), 1);
+ assertEquals(Iterables.getOnlyElement(bundle.getTypes()), entity);
+
+ }
+
+ @Test
+ // different metadata is allowed as that doesn't affect operation (but
different definition is not, see below)
+ // if in same bundle, it's deduped and last one wins; should warn and
could disallow, but if type is pulled in
+ // multiple times from copied files, it feels convenient just to
dedupe and forgive minor metadata changes;
+ // if in different bundles, see other test below, but in that case
both are added
+ public void
testAddSameTypeTwiceInSameBundleDifferentDisplayName_LastOneWins() throws
Exception {
+ final String symbolicName =
"test.duplicate.type."+JavaClassNames.niceClassAndMethod();
+ final String entityName = symbolicName+".type";
+
+ File jar = createJar(ImmutableMap.<String,
String>of("catalog.bom", Joiner.on("\n").join(
+ "brooklyn.catalog:",
+ " bundle: " + symbolicName,
+ " version: " + TEST_VERSION,
+ " items:",
+ " - id: " + entityName,
+ " itemType: entity",
+ " name: T",
+ " item:",
+ " type:
org.apache.brooklyn.core.test.entity.TestEntity",
+ " - id: " + entityName,
+ " itemType: entity",
+ " name: U",
+ " item:",
+ " type:
org.apache.brooklyn.core.test.entity.TestEntity")));
+
+ Response result = client().path("/catalog/bundles")
+ .header(HttpHeaders.CONTENT_TYPE, "application/x-jar")
+ .post(Streams.readFully(new FileInputStream(jar)));
+ HttpAsserts.assertHealthyStatusCode(result.getStatus());
+
+ TypeSummary entity = client().path("/catalog/types/" + entityName
+ "/" + TEST_VERSION)
+ .get(TypeSummary.class);
+ assertEquals(entity.getDisplayName(), "U");
+
+ List<TypeSummary> entities = client().path("/catalog/types/" +
entityName)
+ .get(new GenericType<List<TypeSummary>>() {});
+ Asserts.assertSize(entities, 1);
+ assertEquals(Iterables.getOnlyElement(entities), entity);
+
+ BundleSummary bundle = client().path("/catalog/bundles/" +
symbolicName + "/" + TEST_VERSION)
+ .get(BundleSummary.class);
+ Asserts.assertSize(bundle.getTypes(), 1);
+ assertEquals(Iterables.getOnlyElement(bundle.getTypes()), entity);
+ }
+
+ @Test
+ // would be confusing if the _definition_ is different however, as one
will be ignored
+ public void
testAddSameTypeTwiceInSameBundleDifferentDefinition_Disallowed() throws
Exception {
+ final String symbolicName =
"test.duplicate.type."+JavaClassNames.niceClassAndMethod();
+ final String entityName = symbolicName+".type";
+ final String entityNameOkay = symbolicName+".okayType";
+
+ File jar = createJar(ImmutableMap.<String,
String>of("catalog.bom", Joiner.on("\n").join(
+ "brooklyn.catalog:",
+ " bundle: " + symbolicName,
+ " version: " + TEST_VERSION,
+ " items:",
+ " - id: " + entityNameOkay,
+ " itemType: entity",
+ " item:",
+ " type:
org.apache.brooklyn.core.test.entity.TestEntity",
+ " - id: " + entityName,
+ " itemType: entity",
+ " item:",
+ " type:
org.apache.brooklyn.core.test.entity.TestEntity",
+ " a.config: first_definition",
+ " - id: " + entityName,
+ " itemType: entity",
+ " item:",
+ " type:
org.apache.brooklyn.core.test.entity.TestEntity",
+ " a.config:
second_definition_makes_it_different_so_disallowed")));
+
+ Response result = client().path("/catalog/bundles")
+ .header(HttpHeaders.CONTENT_TYPE, "application/x-jar")
+ .post(Streams.readFully(new FileInputStream(jar)));
+ HttpAsserts.assertNotHealthyStatusCode(result.getStatus());
+ String resultS =
Streams.readFullyString((InputStream)result.getEntity());
+ Asserts.assertStringContainsIgnoreCase(resultS, "different plan",
entityName);
+ Asserts.assertStringDoesNotContain(resultS, entityNameOkay);
+
+ // entity not added
+ Response get = client().path("/catalog/types/" + entityName + "/"
+ TEST_VERSION).get();
+ assertEquals(get.getStatus(), 404);
+
+ List<TypeSummary> entities = client().path("/catalog/types/" +
entityName)
+ .get(new GenericType<List<TypeSummary>>() {});
+ Asserts.assertSize(entities, 0);
+
+ // nor is the okay entity
+ Response getOkay = client().path("/catalog/types/" +
entityNameOkay + "/" + TEST_VERSION).get();
+ assertEquals(getOkay.getStatus(), 404);
+
+ // and nor is the bundle
+ Response getBundle = client().path("/catalog/bundles/" +
symbolicName + "/" + TEST_VERSION).get();
+ assertEquals(getBundle.getStatus(), 404);
+ }
+
+ // TODO might in future want to allow this if the user adding the type
cannot see the other type due to entitlements;
+ // means however there might be another user who _can_ see the two
different types
+ private final static boolean
DISALLOW_DIFFERENCES_IN_SAME_TYPE_ID_FROM_DIFFERENT_BUNDLES = true;
+
+ @Test
+ public void
testAddSameTypeTwiceInDifferentBundleDifferentDefinition_Disallowed() throws
Exception {
--- End diff --
I don't understand why this case is not allowed. Could you elaborate on
this please @ahgittin ?
---
