improve update summary count calculations (DAT-16656 and DAT-16357) (#…

…5634)

* counts improved but still reporting a failure as previously run

* properly calculate total previously run

* remove unused getter

* Update totalInChangelog calculation.

* - Added missing filtered total to the totalInChangelog calculation.
- Test update.

* Added additionalStatus count which will contain the count of failed deployment changesets to the total.

---------

Co-authored-by: Daniel Mallorga <dmallorga@liquibase.com>

liquibase-integration-tests/src/test/resources/liquibase/extension/testing/command/update.test.groovy

@@ -6,7 +6,6 @@ import liquibase.changelog.RanChangeSet
 import liquibase.database.Database
 import liquibase.exception.CommandExecutionException
 import liquibase.exception.CommandValidationException
-import liquibase.extension.testing.setup.SetupEnvironmentVariableProvider
 
 import java.util.regex.Pattern
 
@@ -511,11 +510,11 @@ Optional Args:
                                         [
                                                 """
 UPDATE SUMMARY
-Run:                          1
+Run:                          0
 Previously run:               0
 Filtered out:                 0
 -------------------------------
-Total change sets:            1
+Total change sets:            0
 """
                                         ]
         ]

liquibase-standard/src/main/java/liquibase/changelog/filter/ShouldRunChangeSetFilter.java

@@ -14,6 +14,8 @@ public class ShouldRunChangeSetFilter implements ChangeSetFilter {
 
     private final Map<String, RanChangeSet> ranChangeSets;
     private final boolean ignoreClasspathPrefix;
+    public static final String CHANGESET_ALREADY_RAN_MESSAGE = "Changeset already ran";
+
 
     public ShouldRunChangeSetFilter(Database database, boolean ignoreClasspathPrefix) throws DatabaseException {
         this.ignoreClasspathPrefix = ignoreClasspathPrefix;
@@ -64,7 +66,7 @@ public ChangeSetFilterResult accepts(ChangeSet changeSet) {
             if (changeSet.shouldRunOnChange() && checksumChanged(changeSet, ranChangeSet)) {
                 return new ChangeSetFilterResult(true, "Changeset checksum changed", this.getClass(), getMdcName(), getDisplayName());
             }
-            return new ChangeSetFilterResult(false, "Changeset already ran", this.getClass(), getMdcName(), getDisplayName());
+            return new ChangeSetFilterResult(false, CHANGESET_ALREADY_RAN_MESSAGE, this.getClass(), getMdcName(), getDisplayName());
         }
         return new ChangeSetFilterResult(true, "Changeset has not ran yet", this.getClass(), getMdcName(), getDisplayName());
     }

liquibase-standard/src/main/java/liquibase/command/core/AbstractUpdateCommandStep.java

@@ -111,7 +111,7 @@ public void run(CommandResultsBuilder resultsBuilder) throws Exception {
                     runChangeLogIterator.run(new UpdateVisitor(database, changeExecListener, new ShouldRunChangeSetFilter(database)),
                             new RuntimeEnvironment(database, contexts, labelExpression));
                 } finally {
-                    UpdateSummaryDetails details = ShowSummaryUtil.buildSummaryDetails(databaseChangeLog, getShowSummary(commandScope), getShowSummaryOutput(commandScope), statusVisitor, resultsBuilder.getOutputStream(), runChangeLogIterator);
+                    UpdateSummaryDetails details = ShowSummaryUtil.buildSummaryDetails(databaseChangeLog, getShowSummary(commandScope), getShowSummaryOutput(commandScope), statusVisitor, resultsBuilder.getOutputStream(), runChangeLogIterator, changeExecListener);
                     if (details != null) {
                         updateReportParameters.getOperationInfo().setUpdateSummaryMsg(details.getOutput());
                         updateReportParameters.getChangesetInfo().addAllToPendingChangesetInfoList(details.getSkipped());

liquibase-standard/src/main/java/liquibase/util/ShowSummaryUtil.java

@@ -1,6 +1,9 @@
 package liquibase.util;
 
-import liquibase.*;
+import liquibase.GlobalConfiguration;
+import liquibase.Scope;
+import liquibase.UpdateSummaryEnum;
+import liquibase.UpdateSummaryOutputEnum;
 import liquibase.changelog.ChangeLogIterator;
 import liquibase.changelog.ChangeSet;
 import liquibase.changelog.ChangeSetStatus;
@@ -9,6 +12,8 @@
 import liquibase.changelog.filter.ChangeSetFilterResult;
 import liquibase.changelog.filter.DbmsChangeSetFilter;
 import liquibase.changelog.filter.ShouldRunChangeSetFilter;
+import liquibase.changelog.visitor.ChangeExecListener;
+import liquibase.changelog.visitor.DefaultChangeExecListener;
 import liquibase.changelog.visitor.StatusVisitor;
 import liquibase.exception.LiquibaseException;
 import liquibase.logging.mdc.MdcKey;
@@ -62,22 +67,41 @@ public static void showUpdateSummary(DatabaseChangeLog changeLog, UpdateSummaryE
      */
     public static void showUpdateSummary(DatabaseChangeLog changeLog, UpdateSummaryEnum showSummary, UpdateSummaryOutputEnum showSummaryOutput, StatusVisitor statusVisitor, OutputStream outputStream, ChangeLogIterator runChangeLogIterator)
             throws LiquibaseException, IOException {
-        buildSummaryDetails(changeLog, showSummary, showSummaryOutput, statusVisitor, outputStream, runChangeLogIterator);
+        buildSummaryDetails(changeLog, showSummary, showSummaryOutput, statusVisitor, outputStream, runChangeLogIterator, null);
     }
 
     /**
      * Show a summary of the changesets which were executed AND return an object with the records of what has happened.
      *
-     * @param changeLog         The changelog used in this update
-     * @param showSummary       Flag to control whether we show the summary
-     * @param showSummaryOutput Flag to control where we show the summary
-     * @param statusVisitor     The StatusVisitor used to determine statuses
-     * @param outputStream      The OutputStream to use for the summary
+     * @param changeLog          The changelog used in this update
+     * @param showSummary        Flag to control whether we show the summary
+     * @param showSummaryOutput  Flag to control where we show the summary
+     * @param statusVisitor      The StatusVisitor used to determine statuses
+     * @param outputStream       The OutputStream to use for the summary
+     * @return the details of the update summary
+     * @throws LiquibaseException Thrown by this method
+     * @throws IOException        Thrown by this method
+     * @deprecated use {@link ShowSummaryUtil#buildSummaryDetails(DatabaseChangeLog, UpdateSummaryEnum, UpdateSummaryOutputEnum, StatusVisitor, OutputStream, ChangeLogIterator, ChangeExecListener)} instead.
+     */
+    @Deprecated
+    public static UpdateSummaryDetails buildSummaryDetails(DatabaseChangeLog changeLog, UpdateSummaryEnum showSummary, UpdateSummaryOutputEnum showSummaryOutput, StatusVisitor statusVisitor, OutputStream outputStream, ChangeLogIterator runChangeLogIterator) throws LiquibaseException, IOException {
+        return buildSummaryDetails(changeLog, showSummary, showSummaryOutput, statusVisitor,outputStream, runChangeLogIterator, null);
+    }
+
+    /**
+     * Show a summary of the changesets which were executed AND return an object with the records of what has happened.
+     *
+     * @param changeLog          The changelog used in this update
+     * @param showSummary        Flag to control whether we show the summary
+     * @param showSummaryOutput  Flag to control where we show the summary
+     * @param statusVisitor      The StatusVisitor used to determine statuses
+     * @param outputStream       The OutputStream to use for the summary
+     * @param changeExecListener
      * @return the details of the update summary
      * @throws LiquibaseException Thrown by this method
      * @throws IOException        Thrown by this method
      */
-    public static UpdateSummaryDetails buildSummaryDetails(DatabaseChangeLog changeLog, UpdateSummaryEnum showSummary, UpdateSummaryOutputEnum showSummaryOutput, StatusVisitor statusVisitor, OutputStream outputStream, ChangeLogIterator runChangeLogIterator)
+    public static UpdateSummaryDetails buildSummaryDetails(DatabaseChangeLog changeLog, UpdateSummaryEnum showSummary, UpdateSummaryOutputEnum showSummaryOutput, StatusVisitor statusVisitor, OutputStream outputStream, ChangeLogIterator runChangeLogIterator, ChangeExecListener changeExecListener)
             throws LiquibaseException, IOException {
         //
         // Check the global flag to turn the summary off
@@ -113,7 +137,7 @@ public static UpdateSummaryDetails buildSummaryDetails(DatabaseChangeLog changeL
         //
         // Only show the summary
         //
-        UpdateSummaryDetails summaryDetails = showSummary(changeLog, statusVisitor, skippedChangeSets, filterDenied, outputStream, showSummaryOutput, runChangeLogIterator);
+        UpdateSummaryDetails summaryDetails = showSummary(changeLog, statusVisitor, skippedChangeSets, filterDenied, outputStream, showSummaryOutput, runChangeLogIterator, changeExecListener);
         summaryDetails.getSummary().setValue(showSummary.toString());
         boolean shouldPrintDetailTable = showSummary != UpdateSummaryEnum.SUMMARY && (!skippedChangeSets.isEmpty() || !denied.isEmpty() || !additionalChangeSetStatus.isEmpty());
 
@@ -270,17 +294,19 @@ private static UpdateSummaryDetails showSummary(DatabaseChangeLog changeLog,
                                                     List<ChangeSetStatus> filterDenied,
                                                     OutputStream outputStream,
                                                     UpdateSummaryOutputEnum showSummaryOutput,
-                                                    ChangeLogIterator runChangeLogIterator) throws LiquibaseException {
+                                                    ChangeLogIterator runChangeLogIterator,
+                                                    ChangeExecListener changeExecListener) throws LiquibaseException {
         StringBuilder builder = new StringBuilder();
         builder.append(System.lineSeparator());
-        int totalInChangelog = changeLog.getChangeSets().size() + skippedChangeSets.size();
         int skipped = skippedChangeSets.size();
         int filtered = filterDenied.size();
         ShowSummaryGeneratorFactory showSummaryGeneratorFactory = Scope.getCurrentScope().getSingleton(ShowSummaryGeneratorFactory.class);
         ShowSummaryGenerator showSummaryGenerator = showSummaryGeneratorFactory.getShowSummaryGenerator();
-        int additional = showSummaryGenerator.getAllAdditionalChangeSetStatus(runChangeLogIterator).size();
-        int totalAccepted = statusVisitor.getChangeSetsToRun().size() - additional;
-        int totalPreviouslyRun = totalInChangelog - filtered - skipped - totalAccepted - additional;
+        List<ChangeSetStatus> additionalChangeSetStatus = showSummaryGenerator.getAllAdditionalChangeSetStatus(runChangeLogIterator);
+        int totalAccepted = calculateAccepted(statusVisitor, changeExecListener);
+        int totalPreviouslyRun = calculatePreviouslyRun(statusVisitor);
+        int additionalStatusCount = additionalChangeSetStatus.size();
+        int totalInChangelog = totalAccepted + totalPreviouslyRun + skipped + filtered + additionalStatusCount;
         UpdateSummary updateSummaryMdc = new UpdateSummary(null, totalAccepted, totalPreviouslyRun, null, totalInChangelog);
 
         String message = "UPDATE SUMMARY";
@@ -343,6 +369,28 @@ private static UpdateSummaryDetails showSummary(DatabaseChangeLog changeLog,
         return updateSummaryDetails;
     }
 
+    private static int calculatePreviouslyRun(StatusVisitor statusVisitor) {
+        return (int) statusVisitor.getStatuses().stream().filter(
+                s -> s.getFilterResults().stream().anyMatch(
+                        fr -> fr.getFilter().isAssignableFrom(ShouldRunChangeSetFilter.class) && !fr.isAccepted() && fr.getMessage().equals(ShouldRunChangeSetFilter.CHANGESET_ALREADY_RAN_MESSAGE)
+                )
+        ).count();
+    }
+
+    /**
+     * Calculate the accepted number of changesets.
+     * The status visitor provides a list of changesets that are expected to execute, not the actual list of
+     * executed changesets. We retain this code despite its inaccuracy for backwards compatibility, in case
+     * a change exec listener is not provided.
+     */
+    private static int calculateAccepted(StatusVisitor statusVisitor, ChangeExecListener changeExecListener) {
+        int ran = statusVisitor.getChangeSetsToRun().size();
+        if (changeExecListener instanceof DefaultChangeExecListener) {
+            ran = ((DefaultChangeExecListener) changeExecListener).getDeployedChangeSets().size();
+        }
+        return ran;
+    }
+
     //
     // Create a Writer to display the summary
     //