DPMMA-3118 Segment build time optimization (Segment membership filter) (mautic#15031)

* fix: [DPMMA-3118] segment static filters

fix: [DPMMA-3118] breaking change info and cleanup

* fix: [DPMMA-3118] phpstan and csfixer

* fix: [DPMMA-3118] segment functional tests

---------

Co-authored-by: Ruth Cheesley <ruth@ruthcheesley.co.uk>

Author: patrykgruszka

UPGRADE-7.0.md

@@ -58,6 +58,7 @@
 ### PHP
 - Removed `Mautic\DashboardBundle\Dashboard\Widget::FORMAT_MYSQL` constant. Use `DateTimeHelper::FORMAT_DB_DATE_ONLY` instead.
 - Removed `Mautic\ApiBundle\Security\OAuth2\Firewall::OAuthListener` class as it was empty. Use `FOS\OAuthServerBundle\Security\Firewall\OAuthListener` instead.
+- Removed `Mautic\LeadBundle\Segment\Query\Filter\SegmentReferenceFilterQueryBuilder` as unused.
 
 ### Javascript
 - Removed `Mautic.insertTextInEditor` function. Use `Mautic.insertHtmlInEditor` instead.

app/bundles/LeadBundle/Command/UpdateLeadListsCommand.php

@@ -78,15 +78,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int
     {
         $id                    = $input->getOption('list-id');
         $batch                 = $input->getOption('batch-limit');
-        $max                   = $input->getOption('max-contacts');
+        $max                   = $input->getOption('max-contacts') ? (int) $input->getOption('max-contacts') : null;
         $enableTimeMeasurement = (bool) $input->getOption('timing');
         $output                = ($input->getOption('quiet')) ? new NullOutput() : $output;
         $excludeSegments       = $input->getOption('exclude');
 
-        if (is_numeric($max)) {
-            $max = (int) $max;
-        }
-
         if (!$this->checkRunStatus($input, $output, $id)) {
             return \Symfony\Component\Console\Command\Command::SUCCESS;
         }
@@ -104,7 +100,18 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                 return \Symfony\Component\Console\Command\Command::FAILURE;
             }
 
-            $this->rebuildSegment($list, $batch, $max, $output);
+            // Track already rebuilt lists to avoid rebuilding them multiple times
+            $rebuiltLists = [];
+
+            // First check if this segment has dependencies and rebuild them
+            if ($list->hasFilterTypeOf('leadlist')) {
+                $this->rebuildDependentSegments($list, $rebuiltLists, $batch, $max, $output, $enableTimeMeasurement, [], $excludeSegments);
+            }
+
+            // Add the current list ID to the rebuilt lists to avoid rebuilding it again
+            $rebuiltLists[] = (int) $list->getId();
+
+            $this->rebuildSegment($list, $batch, $max, $output, $enableTimeMeasurement);
         } else {
             $filter = [
                 'iterable_mode' => true,
@@ -121,18 +128,30 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                     ],
                 ];
             }
-            $leadLists = $this->listModel->getEntities($filter);
 
+            $rebuiltLists = [];
+            $leadLists    = $this->listModel->getEntities($filter);
+
+            /** @var LeadList $leadList */
             foreach ($leadLists as $leadList) {
-                $startTimeForSingleSegment = time();
-                $this->rebuildSegment($leadList, $batch, $max, $output);
-                if ($enableTimeMeasurement) {
-                    $totalTime = round(microtime(true) - $startTimeForSingleSegment, 2);
-                    $output->writeln('<fg=cyan>'.$this->translator->trans('mautic.lead.list.rebuild.contacts.time', ['%time%' => $totalTime]).'</>'."\n");
+                $listId = $leadList->getId();
+
+                // Skip if already rebuilt
+                if (in_array($listId, $rebuiltLists)) {
+                    continue;
                 }
-                unset($leadList);
+
+                // Process any dependent segments first (segments that are used as filters in this segment)
+                if ($leadList->hasFilterTypeOf('leadlist')) {
+                    $this->rebuildDependentSegments($leadList, $rebuiltLists, $batch, $max, $output, $enableTimeMeasurement, [], $excludeSegments);
+                }
+
+                // Add the current list ID to the rebuilt lists to avoid rebuilding it again
+                $rebuiltLists[] = $listId;
+
+                // Rebuild the current segment
+                $this->rebuildSegment($leadList, $batch, $max, $output, $enableTimeMeasurement);
             }
-            unset($leadLists);
         }
 
         $this->completeRun();
@@ -145,23 +164,103 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         return \Symfony\Component\Console\Command\Command::SUCCESS;
     }
 
-    private function rebuildSegment(LeadList $segment, int $batch, int $max, OutputInterface $output): void
-    {
-        if ($segment->isPublished()) {
-            $output->writeln('<info>'.$this->translator->trans('mautic.lead.list.rebuild.rebuilding', ['%id%' => $segment->getId()]).'</info>');
-            $startTime   = microtime(true);
-            $processed   = $this->listModel->rebuildListLeads($segment, $batch, $max, $output);
-            $rebuildTime = round(microtime(true) - $startTime, 2);
-            if (0 >= (int) $max) {
-                // Only full segment rebuilds count
-                $segment->setLastBuiltDateToCurrentDatetime();
-                $segment->setLastBuiltTime($rebuildTime);
-                $this->listModel->saveEntity($segment);
+    /**
+     * @param array<int>        $rebuiltLists    List of segment IDs that have already been rebuilt
+     * @param array<int>        $dependencyChain Chain of segment IDs to detect circular dependencies
+     * @param array<int|string> $excludeSegments List of segment IDs to exclude from rebuilding
+     *
+     * @param-out array<int> $rebuiltLists Updated list of segment IDs that have been rebuilt
+     */
+    private function rebuildDependentSegments(
+        LeadList $leadList,
+        array &$rebuiltLists,
+        int $batch,
+        ?int $max,
+        OutputInterface $output,
+        bool $enableTimeMeasurement,
+        array $dependencyChain = [],
+        array $excludeSegments = [],
+    ): void {
+        // Track the current segment in our dependency chain
+        $currentId         = $leadList->getId();
+        $dependencyChain[] = $currentId;
+
+        foreach ($leadList->getFilters() as $filter) {
+            if ('leadlist' === $filter['type']) {
+                foreach ($filter['filter'] ?? [] as $dependentListId) {
+                    $dependentListId = (int) $dependentListId;
+
+                    // Skip if already rebuilt or in exclude list
+                    if (in_array($dependentListId, $rebuiltLists) || in_array($dependentListId, $excludeSegments)) {
+                        continue;
+                    }
+
+                    // Check for circular dependency
+                    if (in_array($dependentListId, $dependencyChain)) {
+                        $output->writeln(
+                            '<error>'.$this->translator->trans(
+                                'Circular dependency detected in segment chain: %chain%',
+                                ['%chain%' => implode(' → ', array_merge($dependencyChain, [$dependentListId]))]
+                            ).'</error>'
+                        );
+                        continue; // Skip this dependency to prevent infinite recursion
+                    }
+
+                    $dependentLeadList = $this->listModel->getEntity($dependentListId);
+                    if (!$dependentLeadList) {
+                        continue; // Skip if the dependent segment doesn't exist - it may have been deleted
+                    }
+
+                    // Check if this dependent segment has its own dependencies
+                    if ($dependentLeadList->hasFilterTypeOf('leadlist')) {
+                        // Recursively process this segment's dependencies first, passing the current chain
+                        $this->rebuildDependentSegments(
+                            $dependentLeadList,
+                            $rebuiltLists,
+                            $batch,
+                            $max,
+                            $output,
+                            $enableTimeMeasurement,
+                            $dependencyChain,
+                            $excludeSegments
+                        );
+                    }
+
+                    // Now rebuild this dependent segment
+                    $this->rebuildSegment($dependentLeadList, $batch, $max, $output, $enableTimeMeasurement);
+                    $rebuiltLists[] = $dependentListId;
+                }
             }
+        }
+    }
 
-            $output->writeln(
-                '<comment>'.$this->translator->trans('mautic.lead.list.rebuild.leads_affected', ['%leads%' => $processed]).'</comment>'
-            );
+    private function rebuildSegment(LeadList $segment, int $batch, ?int $max, OutputInterface $output, bool $enableTimeMeasurement = false): void
+    {
+        if (!$segment->isPublished()) {
+            return;
+        }
+
+        $output->writeln('<info>'.$this->translator->trans('mautic.lead.list.rebuild.rebuilding', ['%id%' => $segment->getId()]).'</info>');
+        $startTime   = microtime(true);
+        $processed   = $this->listModel->rebuildListLeads($segment, $batch, $max, $output);
+        $rebuildTime = round(microtime(true) - $startTime, 2);
+
+        if (0 >= (int) $max) {
+            // Only full segment rebuilds count
+            $segment->setLastBuiltDateToCurrentDatetime();
+            $segment->setLastBuiltTime($rebuildTime);
+            $this->listModel->saveEntity($segment);
+        }
+
+        $output->writeln(
+            '<comment>'.$this->translator->trans('mautic.lead.list.rebuild.leads_affected', ['%leads%' => $processed]).'</comment>'
+        );
+
+        if ($enableTimeMeasurement) {
+            $output->writeln('<fg=cyan>'.$this->translator->trans(
+                'mautic.lead.list.rebuild.contacts.time',
+                ['%time%' => $rebuildTime]
+            ).'</>'."\n");
         }
     }
 }

app/bundles/LeadBundle/Config/config.php

@@ -797,16 +797,6 @@
                 'class'     => Mautic\LeadBundle\Segment\Query\Filter\ComplexRelationValueFilterQueryBuilder::class,
                 'arguments' => ['mautic.lead.model.random_parameter_name', 'event_dispatcher'],
             ],
-            'mautic.lead.query.builder.special.leadlist' => [
-                'class'     => Mautic\LeadBundle\Segment\Query\Filter\SegmentReferenceFilterQueryBuilder::class,
-                'arguments' => [
-                    'mautic.lead.model.random_parameter_name',
-                    'mautic.lead.repository.lead_segment_query_builder',
-                    'doctrine.orm.entity_manager',
-                    'mautic.lead.model.lead_segment_filter_factory',
-                    'event_dispatcher',
-                ],
-            ],
             'mautic.lead.query.builder.channel_click.value' => [
                 'class'     => Mautic\LeadBundle\Segment\Query\Filter\ChannelClickQueryBuilder::class,
                 'arguments' => [

app/bundles/LeadBundle/Segment/Query/Filter/SegmentReferenceFilterQueryBuilder.php

@@ -11,7 +11,6 @@
 use Mautic\LeadBundle\Segment\Query\Filter\ForeignFuncFilterQueryBuilder;
 use Mautic\LeadBundle\Segment\Query\Filter\ForeignValueFilterQueryBuilder;
 use Mautic\LeadBundle\Segment\Query\Filter\IntegrationCampaignFilterQueryBuilder;
-use Mautic\LeadBundle\Segment\Query\Filter\SegmentReferenceFilterQueryBuilder;
 use Mautic\LeadBundle\Segment\Query\Filter\SessionsFilterQueryBuilder;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
@@ -137,7 +136,10 @@ private function setDefaultFilters(): void
             'type' => DoNotContactFilterQueryBuilder::getServiceId(),
         ];
         $this->filters['leadlist']                      = [
-            'type' => SegmentReferenceFilterQueryBuilder::getServiceId(),
+            'type'          => ForeignValueFilterQueryBuilder::getServiceId(),
+            'foreign_table' => 'lead_lists_leads',
+            'field'         => 'leadlist_id',
+            'where'         => 'lead_lists_leads.manually_removed = 0',
         ];
         $this->filters['globalcategory']                = [
             'type'          => ForeignValueFilterQueryBuilder::getServiceId(),