fix(task) #29 #30 #31 #32 #33 SOAP options and headers per call, SoapFault handling, input check; #23 add missing tests

CHANGELOG.md

@@ -2,8 +2,16 @@ Latest
 ------
 
 ### Changes
+* [#23](https://github.com/cleverage/soap-process-bundle/issues/23) Add missing tests: Client (with a fake SoapClient), RequestTask, RequestTransformer, MissingClientException, bundle and DI extension.
 * [#27](https://github.com/cleverage/soap-process-bundle/issues/27) Give the ids of both services in the error on duplicate client codes: the clients are registered by a compiler pass of the bundle, `ClientRegistry::addClient()` gets an optional `$serviceId` argument. Update documentation, add tests.
 
+### Fixes
+* [#29](https://github.com/cleverage/soap-process-bundle/issues/29) Fix RequestTask: keep the SOAP options and headers of the client definition when `soap_call_options` / `soap_call_headers` are not set (they were overwritten by `null`). Update documentation, add tests.
+* [#30](https://github.com/cleverage/soap-process-bundle/issues/30) Fix RequestTask and RequestTransformer: set `soap_call_options` / `soap_call_headers` for the call only (they leaked to the next calls of the client), add these options to the `soap_request` transformer. Update documentation, add tests.
+* [#31](https://github.com/cleverage/soap-process-bundle/issues/31) Fix Client: throw the `SoapFault` of a failed call (it returned `false`), so a method returning `false` is no longer handled as failed, and the `soap_request` transformer fails on a `SoapFault`. Update documentation, add tests.
+* [#32](https://github.com/cleverage/soap-process-bundle/issues/32) Fix Client: a `SoapFault` returned with the `exceptions: false` option makes the call fail (it was returned as a result). Update documentation, add tests.
+* [#33](https://github.com/cleverage/soap-process-bundle/issues/33) Fix RequestTask: throw an explicit `\UnexpectedValueException` on a non-array input (a `TypeError` was triggered); Client: log the notice of the calls handled by a `soapCall<Method>()` override. Update documentation, add tests.
+
 v3.1
 ------
 

docs/reference/client.md

@@ -113,9 +113,8 @@ class CountryInfoClient extends Client
      */
     protected function soapCallCountryName(array $input): mixed
     {
-        $result = $this->doSoapCall('CountryName', $input);
-
-        return false === $result ? false : $result->CountryNameResult;
+        // Throws the SoapFault when the call fails
+        return $this->doSoapCall('CountryName', $input)->CountryNameResult;
     }
 }
 ```
@@ -134,10 +133,11 @@ Notes
 * The `trace` option is always forced to `true` when the `SoapClient` is created, so that the last request and
   response are always available (they are added to the log context of a failed call). Setting `trace: true` in
   `$options` additionally logs them at `debug` level after each successful call.
-* Each generic call is logged at `notice` level (`Soap call '<method>' on '<wsdl>'`). When a `SoapFault` is thrown
-  (i.e. with `exceptions: true`, the default of `SoapClient`), the error is logged at `alert` level, with the last
-  request and response in the log context, and `false` is returned instead of throwing. With `exceptions: false`,
-  `SoapClient` returns the `SoapFault` object, which is then returned as a regular result.
-* A client service is shared by default: the SOAP call options and headers set by a [RequestTask](tasks/request_task.md) remain
-  set on the client for the following calls, including calls made by the
-  [RequestTransformer](transformers/request_transformer.md).
+* Each call is logged at `notice` level (`Soap call '<method>' on '<wsdl>'`), including the ones handled by a
+  `soapCall<Method>()` override. When the call fails with a `SoapFault`, the error is logged at `alert` level, with the
+  last request and response in the log context, then the `SoapFault` is thrown. With the `exceptions: false` option,
+  `SoapClient` returns the `SoapFault` instead of throwing it: it is handled the same way.
+* A client service is shared by default: the SOAP options and headers set with `setSoapOptions()` /
+  `setSoapHeaders()` (e.g. in the `calls` of the service definition) are used by every call. The
+  [RequestTask](tasks/request_task.md) and the [RequestTransformer](transformers/request_transformer.md) set their
+  own `soap_call_options` / `soap_call_headers` for their call only, then restore these values.

docs/reference/tasks/request_task.md

@@ -15,6 +15,8 @@ Accepted inputs
 [SoapClient::__soapCall()](https://www.php.net/manual/en/soapclient.soapcall.php). An empty input (`null`, `[]`, …)
 calls the method without argument.
 
+Any other non-empty input (e.g. a `string`) throws an `\UnexpectedValueException`.
+
 For a document/literal service, the arguments of the method are usually wrapped in a single array, e.g.
 `{ parameters: { sCountryISOCode: FR } }` (in WSDL mode, the keys of the first level are ignored, only the order
 matters).
@@ -85,14 +87,16 @@ get_order:
 Notes
 -----
 
-* When the call fails (the client returns `false`, see [client](../client.md)), the task logs an error
-  `Empty resultset for query`, with the options and the last request and response in the log context, then throws a
-  `RuntimeException` (`Soap call '<method>' on client '<client>' failed`), handled by the `error_strategy`:
+* When the call fails (the client throws a `SoapFault`, see [client](../client.md)), the task logs an error
+  `Empty resultset for query`, with the options, the fault message and the last request and response in the log
+  context, then throws a `RuntimeException` (`Soap call '<method>' on client '<client>' failed`, with the `SoapFault`
+  as previous exception), handled by the `error_strategy`:
   - with `error_strategy: skip`, the `outputs` tasks are skipped and the `error_outputs` tasks receive the input of
     the task
   - with `error_strategy: stop`, the process fails and the console command returns an error code
-* An exception thrown by the client (e.g. a `SoapFault` when the WSDL cannot be loaded, or a
+* A method returning `false` is a successful call: `false` is output.
+* An exception thrown by the client before the call (e.g. a `SoapFault` when the WSDL cannot be loaded, or a
   `MissingClientException` for an unknown `client`) is handled as usual by the `error_strategy`.
-* `soap_call_options` and `soap_call_headers` are set on the client each time the task is executed, including when
-  they are `null`: they overwrite the values configured with `setSoapOptions` / `setSoapHeaders` in the client
-  service definition.
+* `soap_call_options` and `soap_call_headers` are set on the client for the call only, then the previous values are
+  restored: when they are not set (`null`), the values configured with `setSoapOptions` / `setSoapHeaders` in the
+  client service definition are used, and they never leak to the following calls of the client.

docs/reference/transformers/request_transformer.md

@@ -24,15 +24,17 @@ Possible outputs
 
 `mixed`: the result of the SOAP call, usually a `stdClass` built by `SoapClient` from the response.
 
-`false` when the call failed (`SoapFault`), see Notes.
+A failed call (`SoapFault`) throws a `RuntimeException`, see Notes.
 
 Options
 -------
 
-| Code     | Type     | Required | Default | Description                               |
-|----------|----------|:--------:|---------|-------------------------------------------|
-| `client` | `string` |  **X**   |         | Code of the [client](../client.md) to use |
-| `method` | `string` |  **X**   |         | Name of the SOAP method to call           |
+| Code                | Type          | Required | Default | Description                                                                                          |
+|---------------------|---------------|:--------:|---------|------------------------------------------------------------------------------------------------------|
+| `client`            | `string`      |  **X**   |         | Code of the [client](../client.md) to use                                                            |
+| `method`            | `string`      |  **X**   |         | Name of the SOAP method to call                                                                      |
+| `soap_call_options` | `array\|null` |          | `null`  | `$options` of `SoapClient::__soapCall()`, as for the [RequestTask](../tasks/request_task.md#options) |
+| `soap_call_headers` | `array\|null` |          | `null`  | Headers sent with the request, as for the [RequestTask](../tasks/request_task.md#options)            |
 
 Examples
 --------
@@ -61,12 +63,11 @@ property_accessor:
 Notes
 -----
 
-* The transformer does not handle any SOAP call option or header: the values currently set on the client are used
-  (from the `calls` of the client service definition, or from the last [RequestTask](../tasks/request_task.md)
-  executed with the same client).
-* A failed call is logged by the [client](../client.md) and returns `false`, without any exception: check the result
-  (or chain a transformer that fails on `false`, like `property_accessor` above) if the process must not go on
-  silently with a `false` value.
+* `soap_call_options` and `soap_call_headers` are set on the client for the call only, then the previous values are
+  restored: when they are not set, the values of the client service definition (`calls`) are used.
+* A failed call (`SoapFault`) is logged by the [client](../client.md), then the transformer throws a
+  `RuntimeException` (`Soap call '<method>' on client '<client>' failed`). A method returning `false` returns
+  `false`.
 * One SOAP call is made each time the transformer is applied: to avoid calling the service several times with the same
   arguments, wrap it in the
   [cached](https://github.com/cleverage/process-bundle/blob/main/docs/reference/transformers/cached_transformer.md)

src/Client/Client.php

@@ -167,22 +167,22 @@ public function call(string $method, array $input = []): mixed
     {
         $this->initializeSoapClient();
 
+        $this->getLogger()->notice(
+            \sprintf("Soap call '%s' on '%s'", $method, $this->getWsdl())
+        );
+
         $callMethod = \sprintf('soapCall%s', ucfirst($method));
         if (method_exists($this, $callMethod)) {
             return $this->$callMethod($input);
         }
 
-        $this->getLogger()->notice(
-            \sprintf("Soap call '%s' on '%s'", $method, $this->getWsdl())
-        );
-
         return $this->doSoapCall($method, $input);
     }
 
     /**
      * @param array<mixed> $input
      *
-     * @return bool|mixed
+     * @throws \SoapFault when the call fails, after logging it
      */
     protected function doSoapCall(string $method, array $input = []): mixed
     {
@@ -191,14 +191,18 @@ protected function doSoapCall(string $method, array $input = []): mixed
         }
         try {
             $result = $this->getSoapClient()->__soapCall($method, $input, $this->getSoapOptions(), $this->getSoapHeaders());
+            // With the "exceptions: false" option, SoapClient returns the fault instead of throwing it
+            if ($result instanceof \SoapFault) {
+                throw $result;
+            }
         } catch (\SoapFault $e) {
             $this->getLastRequestTrace();
             $this->getLogger()->alert(
                 \sprintf("Soap call '%s' on '%s' failed : %s", $method, $this->getWsdl(), $e->getMessage()),
                 $this->getLastRequestTraceArray()
             );
 
-            return false;
+            throw $e;
         }
 
         $this->getLastRequestTrace();

src/Client/ClientInterface.php

@@ -97,7 +97,7 @@ public function getLastResponseHeaders(): ?string;
      *
      * @param array<mixed> $input
      *
-     * @return bool|mixed
+     * @throws \SoapFault when the call fails
      */
     public function call(string $method, array $input = []): mixed;
 }

src/Client/SoapCallOptionsTrait.php

@@ -0,0 +1,92 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/SoapProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\SoapProcessBundle\Client;
+
+use Symfony\Component\OptionsResolver\Options;
+use Symfony\Component\OptionsResolver\OptionsResolver;
+
+/**
+ * SOAP options and headers of a single call, used by the RequestTask and the soap_request transformer.
+ *
+ * The options and headers are a state of the (shared) client: they are set for the call only, then the previous
+ * ones (e.g. set by the client definition) are restored.
+ */
+trait SoapCallOptionsTrait
+{
+    protected function configureSoapCallOptions(OptionsResolver $resolver): void
+    {
+        $resolver->setDefaults(
+            [
+                'soap_call_options' => null,
+                'soap_call_headers' => null,
+            ]
+        );
+        $resolver->setAllowedTypes('soap_call_options', ['array', 'null']);
+        $resolver->setAllowedTypes('soap_call_headers', ['array', 'null']);
+
+        $resolver->setNormalizer('soap_call_headers', function (Options $options, $headers) {
+            if (null === $headers) {
+                return null;
+            }
+
+            $headerResolver = new OptionsResolver();
+            $this->configureSoapCallHeaderOption($headerResolver);
+
+            $resolvedHeaders = [];
+            /** @var array<string, array<mixed>> $headers */
+            foreach ($headers as $name => $header) {
+                /** @var array{'namespace': string, 'data': array<mixed>} $resolvedHeader */
+                $resolvedHeader = $headerResolver->resolve($header);
+                $resolvedHeaders[] = new \SoapHeader($resolvedHeader['namespace'], $name, $resolvedHeader['data']);
+            }
+
+            return $resolvedHeaders;
+        });
+    }
+
+    protected function configureSoapCallHeaderOption(OptionsResolver $resolver): void
+    {
+        $resolver->setRequired('namespace');
+        $resolver->setRequired('data');
+    }
+
+    /**
+     * @param array<mixed>            $input
+     * @param array<mixed>|null       $soapCallOptions null to keep the options of the client
+     * @param array<\SoapHeader>|null $soapCallHeaders null to keep the headers of the client
+     */
+    protected function callWithSoapOptions(
+        ClientInterface $client,
+        string $method,
+        array $input,
+        ?array $soapCallOptions,
+        ?array $soapCallHeaders,
+    ): mixed {
+        $clientOptions = $client->getSoapOptions();
+        $clientHeaders = $client->getSoapHeaders();
+        if (null !== $soapCallOptions) {
+            $client->setSoapOptions($soapCallOptions);
+        }
+        if (null !== $soapCallHeaders) {
+            $client->setSoapHeaders($soapCallHeaders);
+        }
+
+        try {
+            return $client->call($method, $input);
+        } finally {
+            $client->setSoapOptions($clientOptions);
+            $client->setSoapHeaders($clientHeaders);
+        }
+    }
+}

src/Task/RequestTask.php

@@ -15,9 +15,9 @@
 
 use CleverAge\ProcessBundle\Model\AbstractConfigurableTask;
 use CleverAge\ProcessBundle\Model\ProcessState;
+use CleverAge\SoapProcessBundle\Client\SoapCallOptionsTrait;
 use CleverAge\SoapProcessBundle\Registry\ClientRegistry;
 use Psr\Log\LoggerInterface;
-use Symfony\Component\OptionsResolver\Options;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 
 /**
@@ -30,6 +30,8 @@
  */
 class RequestTask extends AbstractConfigurableTask
 {
+    use SoapCallOptionsTrait;
+
     public function __construct(protected LoggerInterface $logger, protected ClientRegistry $registry)
     {
     }
@@ -41,22 +43,17 @@ public function execute(ProcessState $state): void
 
         $client = $this->registry->getClient($options['client']);
 
-        /** @var array<mixed> $input */
         $input = $state->getInput() ?: [];
+        if (!\is_array($input)) {
+            throw new \UnexpectedValueException(\sprintf('RequestTask expects an array or empty input, %s given', get_debug_type($input)));
+        }
 
-        /** @var array<mixed>|null $soapCallOptions */
-        $soapCallOptions = $this->getOption($state, 'soap_call_options');
-        $client->setSoapOptions($soapCallOptions);
-        /** @var array<\SoapHeader>|null $soapCallHeaders */
-        $soapCallHeaders = $this->getOption($state, 'soap_call_headers');
-        $client->setSoapHeaders($soapCallHeaders);
-
-        $result = $client->call($options['method'], $input);
-
-        // Handle empty results
-        if (false === $result) {
+        try {
+            $result = $this->callWithSoapOptions($client, $options['method'], $input, $options['soap_call_options'], $options['soap_call_headers']);
+        } catch (\SoapFault $e) {
             $logContext = [
                 'options' => $options,
+                'message' => $e->getMessage(),
                 'last_request' => $client->getLastRequest(),
                 'last_request_headers' => $client->getLastRequestHeaders(),
                 'last_response' => $client->getLastResponse(),
@@ -67,7 +64,7 @@ public function execute(ProcessState $state): void
 
             // The process manager applies the error strategy: the process fails with "stop", the error outputs
             // receive the task input with "skip"
-            throw new \RuntimeException(\sprintf("Soap call '%s' on client '%s' failed", $options['method'], $options['client']));
+            throw new \RuntimeException(\sprintf("Soap call '%s' on client '%s' failed", $options['method'], $options['client']), 0, $e);
         }
 
         $state->setOutput($result);
@@ -81,40 +78,9 @@ protected function configureOptions(OptionsResolver $resolver): void
                 'method',
             ]
         );
-        $resolver->setDefaults(
-            [
-                'soap_call_options' => null,
-                'soap_call_headers' => null,
-            ]
-        );
         $resolver->setAllowedTypes('client', ['string']);
         $resolver->setAllowedTypes('method', ['string']);
-        $resolver->setAllowedTypes('soap_call_options', ['array', 'null']);
-        $resolver->setAllowedTypes('soap_call_headers', ['array', 'null']);
 
-        $resolver->setNormalizer('soap_call_headers', function (Options $options, $headers) {
-            if (null === $headers) {
-                return null;
-            }
-
-            $headerResolver = new OptionsResolver();
-            $this->configureSoapCallHeaderOption($headerResolver);
-
-            $resolvedHeaders = [];
-            /** @var array<string, array<mixed>> $headers */
-            foreach ($headers as $name => $header) {
-                /** @var array{'namespace': string, 'data': array<mixed>} $resolvedHeader */
-                $resolvedHeader = $headerResolver->resolve($header);
-                $resolvedHeaders[] = new \SoapHeader($resolvedHeader['namespace'], $name, $resolvedHeader['data']);
-            }
-
-            return $resolvedHeaders;
-        });
-    }
-
-    protected function configureSoapCallHeaderOption(OptionsResolver $resolver): void
-    {
-        $resolver->setRequired('namespace');
-        $resolver->setRequired('data');
+        $this->configureSoapCallOptions($resolver);
     }
 }