Fix failover handling of chunked api commands (#1295)

nilmerg · web-flow · commit cb5dd3f417b3 · 2025-11-12T15:29:13.000+01:00
fixes #1292
diff --git a/application/forms/Command/Object/AcknowledgeProblemForm.php b/application/forms/Command/Object/AcknowledgeProblemForm.php
@@ -20,8 +20,6 @@
 use ipl\Web\FormDecorator\IcingaFormDecorator;
 use ipl\Web\Widget\Icon;
 use Iterator;
-use LimitIterator;
-use NoRewindIterator;
 use Traversable;
 
 use function ipl\Stdlib\iterable_value_first;
@@ -220,9 +218,9 @@ protected function getCommands(Iterator $objects): Traversable
         }
 
         $granted->rewind(); // Forwards the pointer to the first element
-        while ($granted->valid()) {
+        if ($granted->valid()) {
             // Chunk objects to avoid timeouts with large sets
-            yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 250));
+            yield $command->setObjects($granted)->setChunkSize(250);
         }
     }
 }
diff --git a/application/forms/Command/Object/AddCommentForm.php b/application/forms/Command/Object/AddCommentForm.php
@@ -20,8 +20,6 @@
 use ipl\Web\FormDecorator\IcingaFormDecorator;
 use ipl\Web\Widget\Icon;
 use Iterator;
-use LimitIterator;
-use NoRewindIterator;
 use Traversable;
 
 use function ipl\Stdlib\iterable_value_first;
@@ -163,9 +161,9 @@ protected function getCommands(Iterator $objects): Traversable
         }
 
         $granted->rewind(); // Forwards the pointer to the first element
-        while ($granted->valid()) {
+        if ($granted->valid()) {
             // Chunk objects to avoid timeouts with large sets
-            yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 500));
+            yield $command->setObjects($granted)->setChunkSize(500);
         }
     }
 }
diff --git a/application/forms/Command/Object/CheckNowForm.php b/application/forms/Command/Object/CheckNowForm.php
@@ -11,8 +11,6 @@
 use ipl\Orm\Model;
 use ipl\Web\Widget\Icon;
 use Iterator;
-use LimitIterator;
-use NoRewindIterator;
 use Traversable;
 
 class CheckNowForm extends CommandForm
@@ -63,9 +61,9 @@ protected function getCommands(Iterator $objects): Traversable
         $command->setForced();
 
         $granted->rewind(); // Forwards the pointer to the first element
-        while ($granted->valid()) {
+        if ($granted->valid()) {
             // Chunk objects to avoid timeouts with large sets
-            yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 1000));
+            yield $command->setObjects($granted)->setChunkSize(1000);
         }
     }
 }
diff --git a/application/forms/Command/Object/DeleteCommentForm.php b/application/forms/Command/Object/DeleteCommentForm.php
@@ -11,8 +11,6 @@
 use ipl\Orm\Model;
 use ipl\Web\Widget\Icon;
 use Iterator;
-use LimitIterator;
-use NoRewindIterator;
 use Traversable;
 
 class DeleteCommentForm extends CommandForm
@@ -64,9 +62,9 @@ protected function getCommands(Iterator $objects): Traversable
         $command->setAuthor($this->getAuth()->getUser()->getUsername());
 
         $granted->rewind(); // Forwards the pointer to the first element
-        while ($granted->valid()) {
+        if ($granted->valid()) {
             // Chunk objects to avoid timeouts with large sets
-            yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 500));
+            yield $command->setObjects($granted)->setChunkSize(500);
         }
     }
 }
diff --git a/application/forms/Command/Object/DeleteDowntimeForm.php b/application/forms/Command/Object/DeleteDowntimeForm.php
@@ -11,8 +11,6 @@
 use ipl\Orm\Model;
 use ipl\Web\Widget\Icon;
 use Iterator;
-use LimitIterator;
-use NoRewindIterator;
 use Traversable;
 
 class DeleteDowntimeForm extends CommandForm
@@ -77,9 +75,9 @@ protected function getCommands(Iterator $objects): Traversable
         $command->setAuthor($this->getAuth()->getUser()->getUsername());
 
         $granted->rewind(); // Forwards the pointer to the first element
-        while ($granted->valid()) {
+        if ($granted->valid()) {
             // Chunk objects to avoid timeouts with large sets
-            yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 250));
+            yield $command->setObjects($granted)->setChunkSize(250);
         }
     }
 }
diff --git a/application/forms/Command/Object/ProcessCheckResultForm.php b/application/forms/Command/Object/ProcessCheckResultForm.php
@@ -16,8 +16,6 @@
 use ipl\Web\FormDecorator\IcingaFormDecorator;
 use ipl\Web\Widget\Icon;
 use Iterator;
-use LimitIterator;
-use NoRewindIterator;
 use Traversable;
 
 use function ipl\Stdlib\iterable_value_first;
@@ -154,9 +152,9 @@ protected function getCommands(Iterator $objects): Traversable
         $command->setPerformanceData($this->getValue('perfdata'));
 
         $granted->rewind(); // Forwards the pointer to the first element
-        while ($granted->valid()) {
+        if ($granted->valid()) {
             // Chunk objects to avoid timeouts with large sets
-            yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 250));
+            yield $command->setObjects($granted)->setChunkSize(250);
         }
     }
 }
diff --git a/application/forms/Command/Object/RemoveAcknowledgementForm.php b/application/forms/Command/Object/RemoveAcknowledgementForm.php
@@ -12,8 +12,6 @@
 use ipl\Orm\Model;
 use ipl\Web\Widget\Icon;
 use Iterator;
-use LimitIterator;
-use NoRewindIterator;
 use Traversable;
 
 use function ipl\Stdlib\iterable_value_first;
@@ -77,9 +75,9 @@ protected function getCommands(Iterator $objects): Traversable
         $command->setAuthor($this->getAuth()->getUser()->getUsername());
 
         $granted->rewind(); // Forwards the pointer to the first element
-        while ($granted->valid()) {
+        if ($granted->valid()) {
             // Chunk objects to avoid timeouts with large sets
-            yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 250));
+            yield $command->setObjects($granted)->setChunkSize(250);
         }
     }
 }
diff --git a/application/forms/Command/Object/ScheduleCheckForm.php b/application/forms/Command/Object/ScheduleCheckForm.php
@@ -18,8 +18,6 @@
 use ipl\Web\FormDecorator\IcingaFormDecorator;
 use ipl\Web\Widget\Icon;
 use Iterator;
-use LimitIterator;
-use NoRewindIterator;
 use Traversable;
 
 use function ipl\Stdlib\iterable_value_first;
@@ -129,9 +127,9 @@ protected function getCommands(Iterator $objects): Traversable
         $command->setCheckTime($this->getValue('check_time')->getTimestamp());
 
         $granted->rewind(); // Forwards the pointer to the first element
-        while ($granted->valid()) {
+        if ($granted->valid()) {
             // Chunk objects to avoid timeouts with large sets
-            yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 1000));
+            yield $command->setObjects($granted)->setChunkSize(1000);
         }
     }
 }
diff --git a/application/forms/Command/Object/ScheduleServiceDowntimeForm.php b/application/forms/Command/Object/ScheduleServiceDowntimeForm.php
@@ -19,8 +19,6 @@
 use ipl\Web\FormDecorator\IcingaFormDecorator;
 use ipl\Web\Widget\Icon;
 use Iterator;
-use LimitIterator;
-use NoRewindIterator;
 use Traversable;
 
 class ScheduleServiceDowntimeForm extends CommandForm
@@ -290,9 +288,9 @@ protected function getCommands(Iterator $objects): Traversable
         }
 
         $granted->rewind(); // Forwards the pointer to the first element
-        while ($granted->valid()) {
+        if ($granted->valid()) {
             // Chunk objects to avoid timeouts with large sets
-            yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 250));
+            yield $command->setObjects($granted)->setChunkSize(250);
         }
     }
 }
diff --git a/application/forms/Command/Object/SendCustomNotificationForm.php b/application/forms/Command/Object/SendCustomNotificationForm.php
@@ -17,8 +17,6 @@
 use ipl\Web\FormDecorator\IcingaFormDecorator;
 use ipl\Web\Widget\Icon;
 use Iterator;
-use LimitIterator;
-use NoRewindIterator;
 use Traversable;
 
 use function ipl\Stdlib\iterable_value_first;
@@ -130,9 +128,9 @@ protected function getCommands(Iterator $objects): Traversable
         $command->setAuthor($this->getAuth()->getUser()->getUsername());
 
         $granted->rewind(); // Forwards the pointer to the first element
-        while ($granted->valid()) {
+        if ($granted->valid()) {
             // Chunk objects to avoid timeouts with large sets
-            yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 500));
+            yield $command->setObjects($granted)->setChunkSize(500);
         }
     }
 }
diff --git a/application/forms/Command/Object/ToggleObjectFeaturesForm.php b/application/forms/Command/Object/ToggleObjectFeaturesForm.php
@@ -12,8 +12,6 @@
 use ipl\Orm\Model;
 use ipl\Web\FormDecorator\IcingaFormDecorator;
 use Iterator;
-use LimitIterator;
-use NoRewindIterator;
 use Traversable;
 
 class ToggleObjectFeaturesForm extends CommandForm
@@ -181,11 +179,11 @@ protected function getCommands(Iterator $objects): Traversable
             $command->setEnabled((int) $state);
 
             $granted->rewind(); // Forwards the pointer to the first element
-            while ($granted->valid()) {
+            if ($granted->valid()) {
                 $this->submittedFeatures[$command->getFeature()] ??= $command->getEnabled();
 
                 // Chunk objects to avoid timeouts with large sets
-                yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 1000));
+                yield $command->setObjects($granted)->setChunkSize(1000);
             }
         }
     }
diff --git a/library/Icingadb/Command/Object/ObjectsCommand.php b/library/Icingadb/Command/Object/ObjectsCommand.php
@@ -9,6 +9,7 @@
 use Icinga\Module\Icingadb\Command\IcingaCommand;
 use InvalidArgumentException;
 use ipl\Orm\Model;
+use Iterator;
 use LogicException;
 use Traversable;
 
@@ -20,20 +21,27 @@ abstract class ObjectsCommand extends IcingaCommand
     /**
      * Involved objects
      *
-     * @var Traversable<Model>
+     * @var ?Iterator<Model>
      */
     protected $objects;
 
+    /**
+     * How many objects to process at once
+     *
+     * @var ?int
+     */
+    protected ?int $chunkSize = null;
+
     /**
      * Set the involved objects
      *
-     * @param Traversable<Model> $objects Except generators
+     * @param Iterator<Model> $objects Except generators
      *
      * @return $this
      *
      * @throws InvalidArgumentException If a generator is passed
      */
-    public function setObjects(Traversable $objects): self
+    public function setObjects(Iterator $objects): self
     {
         if ($objects instanceof Generator) {
             throw new InvalidArgumentException('Generators are not supported');
@@ -61,9 +69,9 @@ public function setObject(Model $object): self
     /**
      * Get the involved objects
      *
-     * @return Traversable
+     * @return Iterator
      */
-    public function getObjects(): Traversable
+    public function getObjects(): Iterator
     {
         if ($this->objects === null) {
             throw new LogicException(
@@ -73,4 +81,28 @@ public function getObjects(): Traversable
 
         return $this->objects;
     }
+
+    /**
+     * Set how many objects to process at once
+     *
+     * @param ?int $chunkSize
+     *
+     * @return $this
+     */
+    public function setChunkSize(?int $chunkSize): static
+    {
+        $this->chunkSize = $chunkSize;
+
+        return $this;
+    }
+
+    /**
+     * Get how many objects to process at once
+     *
+     * @return ?int
+     */
+    public function getChunkSize(): ?int
+    {
+        return $this->chunkSize;
+    }
 }
diff --git a/library/Icingadb/Command/Transport/ApiCommandTransport.php b/library/Icingadb/Command/Transport/ApiCommandTransport.php
@@ -256,11 +256,11 @@ protected function sendCommand(IcingaApiCommand $command)
                 ), static::SEND_TIMEOUT, $e);
             }
 
-            throw new CommandTransportException(
+            throw (new CommandTransportException(
                 'Can\'t connect to the Icinga 2 API: %u %s',
                 $e->getCode(),
                 $e->getMessage()
-            );
+            ))->setCommand($command);
         }
 
         try {
@@ -299,16 +299,20 @@ protected function sendCommand(IcingaApiCommand $command)
     /**
      * Send the Icinga command over the Icinga 2 API
      *
-     * @param   IcingaCommand   $command
-     * @param   int|null        $now
+     * @param IcingaCommand|IcingaApiCommand $command
+     * @param int|null $now
      *
-     * @throws  CommandTransportException
+     * @throws CommandTransportException
      *
-     * @return  mixed
+     * @return mixed
      */
-    public function send(IcingaCommand $command, int $now = null)
+    public function send(IcingaCommand|IcingaApiCommand $command, int $now = null)
     {
-        return $this->sendCommand($this->renderer->render($command));
+        if ($command instanceof IcingaCommand) {
+            $command = $this->renderer->render($command);
+        }
+
+        return $this->sendCommand($command);
     }
 
     /**
diff --git a/library/Icingadb/Command/Transport/CommandTransport.php b/library/Icingadb/Command/Transport/CommandTransport.php
diff --git a/library/Icingadb/Command/Transport/CommandTransportException.php b/library/Icingadb/Command/Transport/CommandTransportException.php
diff --git a/test/php/Lib/FailoverCommandTransport.php b/test/php/Lib/FailoverCommandTransport.php
diff --git a/test/php/Lib/IntermittentlyFailingCommandTransport.php b/test/php/Lib/IntermittentlyFailingCommandTransport.php
diff --git a/test/php/library/Icingadb/Command/Transport/CommandTransportTest.php b/test/php/library/Icingadb/Command/Transport/CommandTransportTest.php

Original file line number	Diff line number	Diff line change
`@@ -20,8 +20,6 @@`
`20`	`20`	`use ipl\Web\FormDecorator\IcingaFormDecorator;`
`21`	`21`	`use ipl\Web\Widget\Icon;`
`22`	`22`	`use Iterator;`
`23`		`-use LimitIterator;`
`24`		`-use NoRewindIterator;`
`25`	`23`	`use Traversable;`
`26`	`24`
`27`	`25`	`use function ipl\Stdlib\iterable_value_first;`
`@@ -220,9 +218,9 @@ protected function getCommands(Iterator $objects): Traversable`
`220`	`218`	`}`
`221`	`219`
`222`	`220`	`$granted->rewind(); // Forwards the pointer to the first element`
`223`		`- while ($granted->valid()) {`
	`221`	`+ if ($granted->valid()) {`
`224`	`222`	`// Chunk objects to avoid timeouts with large sets`
`225`		`- yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 250));`
	`223`	`+ yield $command->setObjects($granted)->setChunkSize(250);`
`226`	`224`	`}`
`227`	`225`	`}`
`228`	`226`	`}`
Original file line number	Diff line number	Diff line change
`@@ -11,8 +11,6 @@`
`11`	`11`	`use ipl\Orm\Model;`
`12`	`12`	`use ipl\Web\Widget\Icon;`
`13`	`13`	`use Iterator;`
`14`		`-use LimitIterator;`
`15`		`-use NoRewindIterator;`
`16`	`14`	`use Traversable;`
`17`	`15`
`18`	`16`	`class CheckNowForm extends CommandForm`
`@@ -63,9 +61,9 @@ protected function getCommands(Iterator $objects): Traversable`
`63`	`61`	`$command->setForced();`
`64`	`62`
`65`	`63`	`$granted->rewind(); // Forwards the pointer to the first element`
`66`		`- while ($granted->valid()) {`
	`64`	`+ if ($granted->valid()) {`
`67`	`65`	`// Chunk objects to avoid timeouts with large sets`
`68`		`- yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 1000));`
	`66`	`+ yield $command->setObjects($granted)->setChunkSize(1000);`
`69`	`67`	`}`
`70`	`68`	`}`
`71`	`69`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,8 +16,6 @@`
`16`	`16`	`use ipl\Web\FormDecorator\IcingaFormDecorator;`
`17`	`17`	`use ipl\Web\Widget\Icon;`
`18`	`18`	`use Iterator;`
`19`		`-use LimitIterator;`
`20`		`-use NoRewindIterator;`
`21`	`19`	`use Traversable;`
`22`	`20`
`23`	`21`	`use function ipl\Stdlib\iterable_value_first;`
`@@ -154,9 +152,9 @@ protected function getCommands(Iterator $objects): Traversable`
`154`	`152`	`$command->setPerformanceData($this->getValue('perfdata'));`
`155`	`153`
`156`	`154`	`$granted->rewind(); // Forwards the pointer to the first element`
`157`		`- while ($granted->valid()) {`
	`155`	`+ if ($granted->valid()) {`
`158`	`156`	`// Chunk objects to avoid timeouts with large sets`
`159`		`- yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 250));`
	`157`	`+ yield $command->setObjects($granted)->setChunkSize(250);`
`160`	`158`	`}`
`161`	`159`	`}`
`162`	`160`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,8 +12,6 @@`
`12`	`12`	`use ipl\Orm\Model;`
`13`	`13`	`use ipl\Web\Widget\Icon;`
`14`	`14`	`use Iterator;`
`15`		`-use LimitIterator;`
`16`		`-use NoRewindIterator;`
`17`	`15`	`use Traversable;`
`18`	`16`
`19`	`17`	`use function ipl\Stdlib\iterable_value_first;`
`@@ -77,9 +75,9 @@ protected function getCommands(Iterator $objects): Traversable`
`77`	`75`	`$command->setAuthor($this->getAuth()->getUser()->getUsername());`
`78`	`76`
`79`	`77`	`$granted->rewind(); // Forwards the pointer to the first element`
`80`		`- while ($granted->valid()) {`
	`78`	`+ if ($granted->valid()) {`
`81`	`79`	`// Chunk objects to avoid timeouts with large sets`
`82`		`- yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 250));`
	`80`	`+ yield $command->setObjects($granted)->setChunkSize(250);`
`83`	`81`	`}`
`84`	`82`	`}`
`85`	`83`	`}`
Original file line number	Diff line number	Diff line change
`@@ -18,8 +18,6 @@`
`18`	`18`	`use ipl\Web\FormDecorator\IcingaFormDecorator;`
`19`	`19`	`use ipl\Web\Widget\Icon;`
`20`	`20`	`use Iterator;`
`21`		`-use LimitIterator;`
`22`		`-use NoRewindIterator;`
`23`	`21`	`use Traversable;`
`24`	`22`
`25`	`23`	`use function ipl\Stdlib\iterable_value_first;`
`@@ -129,9 +127,9 @@ protected function getCommands(Iterator $objects): Traversable`
`129`	`127`	`$command->setCheckTime($this->getValue('check_time')->getTimestamp());`
`130`	`128`
`131`	`129`	`$granted->rewind(); // Forwards the pointer to the first element`
`132`		`- while ($granted->valid()) {`
	`130`	`+ if ($granted->valid()) {`
`133`	`131`	`// Chunk objects to avoid timeouts with large sets`
`134`		`- yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 1000));`
	`132`	`+ yield $command->setObjects($granted)->setChunkSize(1000);`
`135`	`133`	`}`
`136`	`134`	`}`
`137`	`135`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,8 +19,6 @@`
`19`	`19`	`use ipl\Web\FormDecorator\IcingaFormDecorator;`
`20`	`20`	`use ipl\Web\Widget\Icon;`
`21`	`21`	`use Iterator;`
`22`		`-use LimitIterator;`
`23`		`-use NoRewindIterator;`
`24`	`22`	`use Traversable;`
`25`	`23`
`26`	`24`	`class ScheduleServiceDowntimeForm extends CommandForm`
`@@ -290,9 +288,9 @@ protected function getCommands(Iterator $objects): Traversable`
`290`	`288`	`}`
`291`	`289`
`292`	`290`	`$granted->rewind(); // Forwards the pointer to the first element`
`293`		`- while ($granted->valid()) {`
	`291`	`+ if ($granted->valid()) {`
`294`	`292`	`// Chunk objects to avoid timeouts with large sets`
`295`		`- yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 250));`
	`293`	`+ yield $command->setObjects($granted)->setChunkSize(250);`
`296`	`294`	`}`
`297`	`295`	`}`
`298`	`296`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,8 +17,6 @@`
`17`	`17`	`use ipl\Web\FormDecorator\IcingaFormDecorator;`
`18`	`18`	`use ipl\Web\Widget\Icon;`
`19`	`19`	`use Iterator;`
`20`		`-use LimitIterator;`
`21`		`-use NoRewindIterator;`
`22`	`20`	`use Traversable;`
`23`	`21`
`24`	`22`	`use function ipl\Stdlib\iterable_value_first;`
`@@ -130,9 +128,9 @@ protected function getCommands(Iterator $objects): Traversable`
`130`	`128`	`$command->setAuthor($this->getAuth()->getUser()->getUsername());`
`131`	`129`
`132`	`130`	`$granted->rewind(); // Forwards the pointer to the first element`
`133`		`- while ($granted->valid()) {`
	`131`	`+ if ($granted->valid()) {`
`134`	`132`	`// Chunk objects to avoid timeouts with large sets`
`135`		`- yield $command->setObjects(new LimitIterator(new NoRewindIterator($granted), 0, 500));`
	`133`	`+ yield $command->setObjects($granted)->setChunkSize(500);`
`136`	`134`	`}`
`137`	`135`	`}`
`138`	`136`	`}`