fix notifier

Author: baev

allure-java-commons/src/main/java/io/qameta/allure/AllureLifecycle.java

@@ -61,6 +61,7 @@
 import java.util.function.Consumer;
 
 import static io.qameta.allure.AllureConstants.ATTACHMENT_FILE_SUFFIX;
+import static io.qameta.allure.util.ResultsUtils.firstNonEmpty;
 import static io.qameta.allure.util.ResultsUtils.getStatus;
 import static io.qameta.allure.util.ResultsUtils.getStatusDetails;
 import static io.qameta.allure.util.ServiceLoaderUtils.load;
@@ -230,7 +231,7 @@ public void scheduleTest(final AllureExternalKey key, final TestResult result) {
             LOGGER.warn(KEY_ALREADY_EXISTS, SCHEDULE_TEST, key);
             return;
         }
-        if (isEmpty(result.getUuid())) {
+        if (firstNonEmpty(result.getUuid()).isEmpty()) {
             result.setUuid(UUID.randomUUID().toString());
         }
         notifier.beforeTestSchedule(result);
@@ -584,22 +585,12 @@ public void startStage(final StepResult result) {
      * Closes consecutive open stages on top of the calling thread's stack. A stage with no status is marked passed.
      */
     private void closeOpenStages() {
-        while (true) {
-            final Optional<AllureExternalKey> current = threadContext.getCurrentStep();
-            if (current.isEmpty()) {
-                return;
-            }
-            final Object item = items.get(current.get());
-            if (!(item instanceof StepItem) || !((StepItem) item).stage()) {
-                return;
-            }
-            finalizeStage((StepItem) item);
-            stopStep(current.get(), true);
-        }
+        closeOpenStagesAbove(null);
     }
 
     /**
-     * Closes consecutive open stages bound above the given step on the calling thread's stack.
+     * Closes consecutive open stages bound above the given step on the calling thread's stack, or all consecutive
+     * top stages when the step is {@code null}. A stage with no status is marked passed.
      */
     private void closeOpenStagesAbove(final AllureExternalKey key) {
         while (true) {
@@ -611,17 +602,13 @@ private void closeOpenStagesAbove(final AllureExternalKey key) {
             if (!(item instanceof StepItem) || !((StepItem) item).stage()) {
                 return;
             }
-            finalizeStage((StepItem) item);
+            if (Objects.isNull(((StepItem) item).result().getStatus())) {
+                ((StepItem) item).result().setStatus(Status.PASSED);
+            }
             stopStep(current.get(), true);
         }
     }
 
-    private static void finalizeStage(final StepItem item) {
-        if (Objects.isNull(item.result().getStatus())) {
-            item.result().setStatus(Status.PASSED);
-        }
-    }
-
     /**
      * Updates step by specified key.
      *
@@ -837,7 +824,7 @@ private CompletableFuture<Void> addAttachmentAsync(final AllureExternalKey key,
 
     /**
      * Adds an attachment wrapped in its own instant step under the current executable — the default representation
-     * for user-facing attachments. Falls back to a plain attachment while a lifecycle listener callback is running.
+     * for user-facing attachments. Safe to call from listener callbacks: the wrapper step emits no further events.
      * Takes no effect if no executable is running.
      *
      * @param name    the name of attachment
@@ -852,10 +839,6 @@ public void addAttachmentStep(final String name, final String type,
             LOGGER.warn(NO_CONTEXT_FOR_ATTACHMENT);
             return;
         }
-        if (LifecycleNotifier.isListenerCallbackRunning()) {
-            addAttachment(current.get(), name, type, content, options);
-            return;
-        }
         final AllureExternalKey key = AllureExternalKey.random(AllureLifecycle.class);
         startStep(current.get(), key, new StepResult().setName(attachmentStepName(name)), true);
         if (!items.containsKey(key)) {
@@ -876,8 +859,7 @@ public void addAttachmentStep(final String name, final String type,
 
     /**
      * Adds an attachment wrapped in its own instant step under the specified parent — the default representation
-     * for user-facing attachments. Pure manual linkage: no thread state is touched. Falls back to a plain
-     * attachment while a lifecycle listener callback is running.
+     * for user-facing attachments. Pure manual linkage: no thread state is touched.
      *
      * @param parentKey the external parent key
      * @param name      the name of attachment
@@ -887,11 +869,6 @@ public void addAttachmentStep(final String name, final String type,
      */
     public void addAttachmentStep(final AllureExternalKey parentKey, final String name, final String type,
                                   final InputStream content, final AttachmentOptions options) {
-        if (LifecycleNotifier.isListenerCallbackRunning()) {
-            // a listener adding attachments must not spawn steps (and further listener callbacks) recursively
-            addAttachment(parentKey, name, type, content, options);
-            return;
-        }
         final AllureExternalKey key = AllureExternalKey.random(AllureLifecycle.class);
         startStep(parentKey, key, new StepResult().setName(attachmentStepName(name)), false);
         if (!items.containsKey(key)) {
@@ -913,8 +890,7 @@ public void addAttachmentStep(final AllureExternalKey parentKey, final String na
     /**
      * Adds an async attachment wrapped in its own instant step under the current executable. The attachment content
      * is awaited before the owning test or scope is written; a failed body marks the step broken before the owner
-     * is serialized. Falls back to a plain async attachment while a lifecycle listener callback is running. Takes
-     * no effect if no executable is running.
+     * is serialized. Takes no effect if no executable is running.
      *
      * @param name    the name of attachment
      * @param type    the content type of attachment
@@ -930,9 +906,6 @@ public CompletableFuture<Void> addAttachmentStepAsync(final String name, final S
             LOGGER.warn(NO_CONTEXT_FOR_ATTACHMENT);
             return CompletableFuture.completedFuture(null);
         }
-        if (LifecycleNotifier.isListenerCallbackRunning()) {
-            return addAttachmentAsync(current.get(), name, type, body, options, null);
-        }
         final AllureExternalKey key = AllureExternalKey.random(AllureLifecycle.class);
         final StepResult step = new StepResult()
                 .setName(attachmentStepName(name))
@@ -958,8 +931,7 @@ public CompletableFuture<Void> addAttachmentStepAsync(final String name, final S
     /**
      * Adds an async attachment wrapped in its own instant step under the specified parent. Pure manual linkage: no
      * thread state is touched. The attachment content is awaited before the owning test or scope is written; a
-     * failed body marks the step broken before the owner is serialized. Falls back to a plain async attachment
-     * while a lifecycle listener callback is running.
+     * failed body marks the step broken before the owner is serialized.
      *
      * @param parentKey the external parent key
      * @param name      the name of attachment
@@ -972,9 +944,6 @@ public CompletableFuture<Void> addAttachmentStepAsync(final AllureExternalKey pa
                                                           final String type,
                                                           final CompletionStage<? extends InputStream> body,
                                                           final AttachmentOptions options) {
-        if (LifecycleNotifier.isListenerCallbackRunning()) {
-            return addAttachmentAsync(parentKey, name, type, body, options);
-        }
         final AllureExternalKey key = AllureExternalKey.random(AllureLifecycle.class);
         final StepResult step = new StepResult()
                 .setName(attachmentStepName(name))
@@ -998,7 +967,7 @@ public CompletableFuture<Void> addAttachmentStepAsync(final AllureExternalKey pa
     }
 
     private static String attachmentStepName(final String name) {
-        return isEmpty(name) ? "Attachment" : name;
+        return firstNonEmpty(name).orElse("Attachment");
     }
 
     private Optional<String> addAttachmentLink(final AllureExternalKey key, final String name,
@@ -1011,8 +980,8 @@ private Optional<String> addAttachmentLink(final AllureExternalKey key, final St
             return Optional.empty();
         }
         final Attachment attachment = new Attachment()
-                .setName(isEmpty(name) ? null : name)
-                .setType(isEmpty(type) ? null : type)
+                .setName(firstNonEmpty(name).orElse(null))
+                .setType(firstNonEmpty(type).orElse(null))
                 .setSource(createAttachmentSource(type, options));
         synchronized (item) {
             ((WithAttachments) model).getAttachments().add(attachment);
@@ -1268,7 +1237,7 @@ private void sweepOwnedSteps(final AllureExternalKey ownerKey) {
     }
 
     private void addChild(final ScopeItem scope, final String childUuid) {
-        if (isEmpty(childUuid)) {
+        if (firstNonEmpty(childUuid).isEmpty()) {
             return;
         }
         synchronized (scope) {
@@ -1327,19 +1296,11 @@ private static TestResultContainer toScopeContainer(final ScopeResult scope) {
      * detaches any list alias the consumer may have retained.
      */
     private static void normalizeScope(final ScopeResult scope) {
-        scope.setTests(mutableList(scope.getTests()));
-        scope.setFixtures(mutableList(scope.getFixtures()));
-        scope.setLabels(mutableList(scope.getLabels()));
-        scope.setLinks(mutableList(scope.getLinks()));
-        scope.setParameters(mutableList(scope.getParameters()));
-    }
-
-    private static <T> List<T> mutableList(final List<T> values) {
-        return Objects.isNull(values) ? new ArrayList<>() : new ArrayList<>(values);
-    }
-
-    private static boolean isEmpty(final String s) {
-        return Objects.isNull(s) || s.isEmpty();
+        scope.setTests(new ArrayList<>(Objects.requireNonNullElse(scope.getTests(), List.of())));
+        scope.setFixtures(new ArrayList<>(Objects.requireNonNullElse(scope.getFixtures(), List.of())));
+        scope.setLabels(new ArrayList<>(Objects.requireNonNullElse(scope.getLabels(), List.of())));
+        scope.setLinks(new ArrayList<>(Objects.requireNonNullElse(scope.getLinks(), List.of())));
+        scope.setParameters(new ArrayList<>(Objects.requireNonNullElse(scope.getParameters(), List.of())));
     }
 
     // ── Items ────────────────────────────────────────────────────────────────────────────────

allure-java-commons/src/main/java/io/qameta/allure/listener/LifecycleNotifier.java

@@ -26,6 +26,12 @@
 import java.util.function.BiConsumer;
 
 /**
+ * Dispatches lifecycle events to the registered listeners.
+ *
+ * <p>Notification is not re-entrant: lifecycle operations triggered from inside a listener callback mutate the
+ * model normally but emit no further events. This lets listeners enrich results with the regular API — steps,
+ * stages, attachment steps — without recursing back into themselves.</p>
+ *
  * @since 2.0
  */
 @SuppressWarnings("PMD.TooManyMethods")
@@ -207,29 +213,25 @@ public void afterStepStop(final StepResult result) {
         runSafely(stepListeners, StepLifecycleListener::afterStepStop, result);
     }
 
-    /**
-     * Returns whether a lifecycle listener callback is running on the current thread.
-     *
-     * @return true if a lifecycle listener callback is running, false otherwise
-     */
-    public static boolean isListenerCallbackRunning() {
-        return LISTENER_CALLBACK_RUNNING.get();
-    }
-
     protected <T extends LifecycleListener, S> void runSafely(final List<T> listeners,
                                                               final BiConsumer<T, S> method,
                                                               final S object) {
-        listeners.forEach(listener -> {
-            final boolean previous = LISTENER_CALLBACK_RUNNING.get();
-            LISTENER_CALLBACK_RUNNING.set(true);
-            try {
-                method.accept(listener, object);
-            } catch (Exception e) {
-                LOGGER.error("Could not invoke listener method", e);
-            } finally {
-                LISTENER_CALLBACK_RUNNING.set(previous);
-            }
-        });
+        if (LISTENER_CALLBACK_RUNNING.get()) {
+            // operations triggered from inside a listener callback emit no further events
+            return;
+        }
+        LISTENER_CALLBACK_RUNNING.set(true);
+        try {
+            listeners.forEach(listener -> {
+                try {
+                    method.accept(listener, object);
+                } catch (Exception e) {
+                    LOGGER.error("Could not invoke listener method", e);
+                }
+            });
+        } finally {
+            LISTENER_CALLBACK_RUNNING.set(false);
+        }
     }
 
 }

allure-java-commons/src/test/java/io/qameta/allure/StepLifecycleListenerTest.java

@@ -69,22 +69,25 @@ public void afterStepStart(final StepResult result) {
                 .extracting(StepResult::getName)
                 .containsExactly("first", "second");
 
-        assertThat(run.getTestResults())
-                .flatExtracting(TestResult::getSteps)
-                .filteredOn("name", "first")
-                .flatExtracting(StepResult::getAttachments)
-                .extracting(Attachment::getName)
-                .containsExactly("inner first");
+        // the listener's attachment gets the default representation — an attachment step —
+        // whose own events are suppressed by the notifier
+        assertAttachmentStep(run, "first", "inner first");
+        assertAttachmentStep(run, "second", "inner second");
+
+        assertThat(executionCount.get())
+                .isEqualTo(2);
+    }
 
+    private static void assertAttachmentStep(final AllureResults run, final String stepName,
+                                             final String attachmentName) {
         assertThat(run.getTestResults())
                 .flatExtracting(TestResult::getSteps)
-                .filteredOn("name", "second")
+                .filteredOn("name", stepName)
+                .flatExtracting(StepResult::getSteps)
+                .filteredOn("name", attachmentName)
                 .flatExtracting(StepResult::getAttachments)
                 .extracting(Attachment::getName)
-                .containsExactly("inner second");
-
-        assertThat(executionCount.get())
-                .isEqualTo(2);
+                .containsExactly(attachmentName);
     }
 
     @Issue("177")
@@ -105,19 +108,8 @@ public void beforeStepStop(final StepResult result) {
                 .extracting(StepResult::getName)
                 .containsExactly("first", "second");
 
-        assertThat(run.getTestResults())
-                .flatExtracting(TestResult::getSteps)
-                .filteredOn("name", "first")
-                .flatExtracting(StepResult::getAttachments)
-                .extracting(Attachment::getName)
-                .containsExactly("inner first");
-
-        assertThat(run.getTestResults())
-                .flatExtracting(TestResult::getSteps)
-                .filteredOn("name", "second")
-                .flatExtracting(StepResult::getAttachments)
-                .extracting(Attachment::getName)
-                .containsExactly("inner second");
+        assertAttachmentStep(run, "first", "inner first");
+        assertAttachmentStep(run, "second", "inner second");
 
         assertThat(executionCount.get())
                 .isEqualTo(2);