Skip to content

Breaking Change: setParams Function in AbstractFunction Enforces Mandatory Parameters, Causes Incompatibility with Optional SAP Parameters #25

@ynnoig

Description

@ynnoig

The recent implementation of the setParams function in the \phpsap\classes\AbstractFunction class introduces a breaking change that causes compatibility issues when handling optional SAP parameters.

Problem:

The current setParams function mandates that all keys defined in getAllowedKeys() must be present in the provided $params array. If any key is missing, the function throws an InvalidArgumentException. This behavior does not align with SAP's design, where input parameters can be optional.

Affected Code:

/**
 * Extract all expected SAP remote function call parameters from the given array
 * and set them.
 * @param array<string, null|bool|int|float|string|array<int|string, mixed>> $params An array of SAP remote function call parameters.
 * @return $this
 * @throws ConnectionFailedException
 * @throws IncompleteConfigException
 * @throws InvalidArgumentException
 * @throws UnknownFunctionException
 */
public function setParams(array $params): IFunction
{
    foreach ($this->getAllowedKeys() as $key) {
        if (!array_key_exists($key, $params)) {
            throw new InvalidArgumentException(sprintf(
                '%s is missing parameter key %s!',
                static::class,
                $key
            ));
        }
        $this->set($key, $params[$key]);
    }
    return $this;
}

Previous Implementation:

The previous implementation in the setMultiple function did not enforce this strict requirement and allowed for optional parameters, which is more aligned with SAP's behavior.

/**
 * This method extracts only allowed keys from the given array. This way it
 * behaves differently than the set() method. This method will never throw an
 * exception because of an invalid key.
 * @param array $data Associative array of keys and values to set.
 * @throws InvalidArgumentException
 */
protected function setMultiple(array $data)
{
    foreach ($this->getAllowedKeys() as $key) {
        if (array_key_exists($key, $data)) {
            $this->setValue($key, $data[$key]);
        }
    }
}

Impact:

This change disrupts the expected behavior where input parameters to an SAP remote function call can be optional. The strict enforcement of mandatory parameters leads to unexpected exceptions, breaking the functionality of existing implementations that rely on optional parameters.

Suggestion:

Consider reverting to the previous behavior or modifying the setParams method to accommodate optional parameters, ensuring compatibility with SAP's design.

If desired, I can create a pull request to address this issue. Please let me know.

Thank u in advance!

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions