From 4db4b8fc2593b5730303f2f70730fb8e3548284a Mon Sep 17 00:00:00 2001 From: l-1sqared <30831153+l-1squared@users.noreply.github.com> Date: Wed, 1 Jun 2022 07:40:03 +0200 Subject: [PATCH] Issue-868: fix too many parents for tags Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com> --- .../jgiven/impl/ScenarioModelBuilder.java | 2 +- .../tngtech/jgiven/impl/tag/ResolvedTags.java | 16 ++++--- .../tngtech/jgiven/impl/tag/TagCreator.java | 45 +++++++++---------- .../jgiven/report/model/ReportModel.java | 6 +-- .../jgiven/impl/tag/ResolvedTagsTest.java | 2 +- .../jgiven/impl/tag/TagCreatorTest.java | 28 +++++++----- 6 files changed, 51 insertions(+), 48 deletions(-) diff --git a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/ScenarioModelBuilder.java b/jgiven-core/src/main/java/com/tngtech/jgiven/impl/ScenarioModelBuilder.java index 25fe58eecd3..f945903ea1d 100644 --- a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/ScenarioModelBuilder.java +++ b/jgiven-core/src/main/java/com/tngtech/jgiven/impl/ScenarioModelBuilder.java @@ -504,7 +504,7 @@ private void addTags(ResolvedTags tags) { if (reportModel != null) { this.reportModel.addTags(tags.getDeclaredTags()); //The report model needs to declare the parent tags in a tag map, or the tags cannot be displayed. - this.reportModel.addTags(tags.getParents()); + this.reportModel.addTags(tags.getAncestors()); } if (scenarioModel != null) { diff --git a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/tag/ResolvedTags.java b/jgiven-core/src/main/java/com/tngtech/jgiven/impl/tag/ResolvedTags.java index a52dcf67550..bba8ad8e538 100644 --- a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/tag/ResolvedTags.java +++ b/jgiven-core/src/main/java/com/tngtech/jgiven/impl/tag/ResolvedTags.java @@ -3,6 +3,7 @@ import com.tngtech.jgiven.report.model.Tag; import java.util.ArrayList; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; /** @@ -15,8 +16,11 @@ public List getDeclaredTags() { return resolvedTags.stream().map(resolvedTag -> resolvedTag.tag).collect(Collectors.toList()); } - public List getParents() { - return resolvedTags.stream().flatMap(resolvedTag -> resolvedTag.parents.stream()).collect(Collectors.toList()); + @SuppressWarnings("CheckStyle") + public Set getAncestors() { + return resolvedTags.stream() + .flatMap(resolvedTag -> resolvedTag.ancestors.stream()) + .collect(Collectors.toSet()); } public boolean isEmpty() { @@ -24,16 +28,16 @@ public boolean isEmpty() { } /** - * A single tag declared for a scenario and the parents necessary to display it. + * A single tag declared for a scenario and the ancestors necessary to display it. */ public static class ResolvedTag { public final Tag tag; - public final List parents; + public final List ancestors; - ResolvedTag(Tag tag, List parents) { + ResolvedTag(Tag tag, List ancestors) { this.tag = tag; - this.parents = parents; + this.ancestors = ancestors; } } diff --git a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/tag/TagCreator.java b/jgiven-core/src/main/java/com/tngtech/jgiven/impl/tag/TagCreator.java index 183d8bd4eb1..af60cec3a43 100644 --- a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/tag/TagCreator.java +++ b/jgiven-core/src/main/java/com/tngtech/jgiven/impl/tag/TagCreator.java @@ -49,14 +49,14 @@ public ResolvedTags toTags(Class annotationClass, String.. return new ResolvedTags(); } - List parents = getTags(annotationClass); + List ancestors = getAllAncestorTags(annotationClass); if (values.length > 0) { List explodedTags = getExplodedTags(Iterables.getOnlyElement(tags), values, null, tagConfig); return explodedTags.stream() - .map(tag -> new ResolvedTags.ResolvedTag(tag, parents)) + .map(tag -> new ResolvedTags.ResolvedTag(tag, ancestors)) .collect(new TagCollector()); } else { - return ResolvedTags.from(new ResolvedTags.ResolvedTag(Iterables.getOnlyElement(tags), parents)); + return ResolvedTags.from(new ResolvedTags.ResolvedTag(Iterables.getOnlyElement(tags), ancestors)); } } @@ -72,7 +72,7 @@ public ResolvedTags toTags(Annotation annotation) { } List tags = processConfiguredAnnotation(tagConfig, annotation); - List parents = getTags(annotationType); + List parents = getAllAncestorTags(annotationType); return tags.stream() .map(tag -> new ResolvedTags.ResolvedTag(tag, parents)) .collect(new TagCollector()); @@ -178,39 +178,34 @@ private TagConfiguration fromIsTag(IsTag isTag, Class anno .cssClass(isTag.cssClass()) .color(isTag.color()) .style(isTag.style()) - .tags(getTagNames(annotationType)) + .tags(getNamesOfParentTags(annotationType)) .href(isTag.href()) .hrefGenerator(isTag.hrefGenerator()) .showInNavigation(isTag.showInNavigation()) .build(); } - //TODO consolidate with below - //TODO better naming - private List getTagNames(Class annotationType) { - return Arrays.stream(annotationType.getAnnotations()) - .filter(a -> a.annotationType().isAnnotationPresent(IsTag.class)) - .map(this::toTags) + private List getNamesOfParentTags(Class annotationType) { + return getTagAnnotationsOn(annotationType) .flatMap(resolvedTags -> resolvedTags.getDeclaredTags().stream()) .map(Tag::toIdString) .collect(Collectors.toList()); } - private List getTags(Class annotationType) { - List allTags = Lists.newArrayList(); - - for (Annotation annotation : annotationType.getAnnotations()) { - if (annotation.annotationType().isAnnotationPresent(IsTag.class)) { - allTags.addAll(toTags(annotation).resolvedTags.stream() - .flatMap(tag -> { - Stream tagStream = Stream.of(tag.tag); - return Stream.concat(tagStream, tag.parents.stream()); - }) - .collect(Collectors.toList())); - } - } + private List getAllAncestorTags(Class annotationType) { + return getTagAnnotationsOn(annotationType) + .flatMap(resolvedTags -> resolvedTags.resolvedTags.stream()) + .flatMap(tag -> { + Stream tagStream = Stream.of(tag.tag); + return Stream.concat(tagStream, tag.ancestors.stream()); + }) + .collect(Collectors.toList()); + } - return allTags; + private Stream getTagAnnotationsOn(Class annotationType) { + return Arrays.stream(annotationType.getAnnotations()) + .filter(a -> a.annotationType().isAnnotationPresent(IsTag.class)) + .map(this::toTags); } private List toStringList(Object[] value) { diff --git a/jgiven-core/src/main/java/com/tngtech/jgiven/report/model/ReportModel.java b/jgiven-core/src/main/java/com/tngtech/jgiven/report/model/ReportModel.java index e5fd92df203..89d5b29e359 100644 --- a/jgiven-core/src/main/java/com/tngtech/jgiven/report/model/ReportModel.java +++ b/jgiven-core/src/main/java/com/tngtech/jgiven/report/model/ReportModel.java @@ -137,10 +137,8 @@ public synchronized void addTag(Tag tag) { this.tagMap.put(tag.toIdString(), tag); } - public synchronized void addTags(List tags) { - for (Tag tag : tags) { - addTag(tag); - } + public synchronized void addTags(Iterable tags) { + tags.forEach(this::addTag); } public synchronized Tag getTagWithId(String tagId) { diff --git a/jgiven-core/src/test/java/com/tngtech/jgiven/impl/tag/ResolvedTagsTest.java b/jgiven-core/src/test/java/com/tngtech/jgiven/impl/tag/ResolvedTagsTest.java index 4f744a6a2fe..cc8fae2cd39 100644 --- a/jgiven-core/src/test/java/com/tngtech/jgiven/impl/tag/ResolvedTagsTest.java +++ b/jgiven-core/src/test/java/com/tngtech/jgiven/impl/tag/ResolvedTagsTest.java @@ -18,7 +18,7 @@ public void testResolvedTagsFiltersForDirectTags() { @Test public void testResolvedTagsFiltersForParents() { ResolvedTags underTest = TestTagGenerator.getEnumeratedResolvedTags(5); - assertThat(underTest.getParents()).extracting(Tag::getFullType) + assertThat(underTest.getAncestors()).extracting(Tag::getFullType) .containsExactly("parent1", "parent2", "parent3", "parent4", "parent5"); } } diff --git a/jgiven-core/src/test/java/com/tngtech/jgiven/impl/tag/TagCreatorTest.java b/jgiven-core/src/test/java/com/tngtech/jgiven/impl/tag/TagCreatorTest.java index 5f348b26a6a..ad78ed9ed41 100644 --- a/jgiven-core/src/test/java/com/tngtech/jgiven/impl/tag/TagCreatorTest.java +++ b/jgiven-core/src/test/java/com/tngtech/jgiven/impl/tag/TagCreatorTest.java @@ -30,7 +30,7 @@ public void addLogInterceptor() { @Test public void testAnnotationParsing() { Tag tag = getOnlyTagFor(AnnotationTestClass.class.getAnnotations()[0]); - assertThat(tag.getName()).isEqualTo("AnnotationWithoutValue"); + assertThat(tag.getName()).isEqualTo(AnnotationWithoutValue.class.getSimpleName()); assertThat(tag.getValues()).isEmpty(); assertThat(interceptor.containsLoggingEvent(record -> record.getLevel() == Level.SEVERE)) .as("Attempt to convert an annotation without value method results in an error log") @@ -40,7 +40,7 @@ public void testAnnotationParsing() { @Test public void testAnnotationWithValueParsing() { Tag tag = getOnlyTagFor(AnnotationWithSingleValueTestClass.class.getAnnotations()[0]); - assertThat(tag.getName()).isEqualTo("AnnotationWithSingleValue"); + assertThat(tag.getName()).isEqualTo(AnnotationWithSingleValue.class.getSimpleName()); assertThat(tag.getValues()).containsExactly("testvalue"); } @@ -55,7 +55,7 @@ public void testAnnotationWithName() { @Test public void testAnnotationWithIgnoredValueParsing() { Tag tag = getOnlyTagFor(AnnotationWithIgnoredValueTestClass.class.getAnnotations()[0]); - assertThat(tag.getName()).isEqualTo("AnnotationWithIgnoredValue"); + assertThat(tag.getName()).isEqualTo(AnnotationWithIgnoredValue.class.getSimpleName()); assertThat(tag.getValues()).isEmpty(); assertThat(tag.toIdString()).isEqualTo(AnnotationWithIgnoredValue.class.getName()); } @@ -63,7 +63,7 @@ public void testAnnotationWithIgnoredValueParsing() { @Test public void testAnnotationWithoutExplodedArrayParsing() { Tag tag = getOnlyTagFor(AnnotationWithoutExplodedArrayValueTestClass.class.getAnnotations()[0]); - assertThat(tag.getName()).isEqualTo("AnnotationWithoutExplodedArray"); + assertThat(tag.getName()).isEqualTo(AnnotationWithoutExplodedArray.class.getSimpleName()); assertThat(tag.getValues()).containsExactly("foo", "bar"); } @@ -86,7 +86,7 @@ public void testAnnotationWithParentTag() { Tag tag = getOnlyTagFor(AnnotationWithParentTag.class.getAnnotations()[0]); assertThat(tag.getTags()).containsAll(Arrays.asList( ParentTag.class.getName(), - ParentTagWithValue.class.getPackage().getName() + ".ParentTagWithValue-SomeValue") + ParentTagWithValue.class.getName() + "-SomeValue") ); } @@ -103,17 +103,23 @@ public void testAnnotationWithArrayParsing() { @Test public void testAllParentsOfTagAreResolved() { + String[] expectedNames = Stream.of(TagWithParentTags.class, ParentTag.class, ParentTagWithValue.class) + .map(Class::getSimpleName).toArray(String[]::new); + + ResolvedTags resolvedTags = underTest.toTags(TagWithGrandparentTags.class); + + assertThat(resolvedTags.getAncestors()).extracting(Tag::getName).containsExactlyInAnyOrder(expectedNames); + assertThat(resolvedTags.getDeclaredTags()).extracting(Tag::getName) + .containsExactly(TagWithGrandparentTags.class.getSimpleName()); + } + + @Test + public void testTagConfigurationOnlyRefersToTheTagsSingleParent() { ResolvedTags resolvedTags = underTest.toTags(TagWithGrandparentTags.class); Stream parents = resolvedTags.getDeclaredTags().stream() .map(Tag::getTags) .flatMap(List::stream); assertThat(parents).containsExactly(TagWithParentTags.class.getName()); - assertThat(resolvedTags.getDeclaredTags()) - .extracting(Tag::getName) - .containsExactly("TagWithGrandparentTags"); - assertThat(resolvedTags.getParents()) - .extracting(Tag::getName) - .containsExactlyInAnyOrder("TagWithParentTags", "ParentTag", "ParentTagWithValue"); } private Tag getOnlyTagFor(Annotation annotation) {