Improve custom query options: add merging, validation, and comprehensive documentation (#181)

Copilot · anderly · web-flow · commit 967b0b67188c · 2025-09-04T10:55:10.000-05:00
* Initial plan

* Improve custom query options: add merging, validation, tests, and documentation

Co-authored-by: anderly &lt;573151+anderly@users.noreply.github.com&gt;

* Add validation for '@' character in custom option keys per OData specification

Co-authored-by: anderly &lt;573151+anderly@users.noreply.github.com&gt;

* Fix CustomOptionsTest: provide required parameter for ODataResponse constructor

Co-authored-by: anderly &lt;573151+anderly@users.noreply.github.com&gt;

* Fix CustomOptionsTest: Mock PSR-7 ResponseInterface instead of ODataResponse

- Fix type mismatch error in CustomOptionsTest by updating the mock to return PSR-7 ResponseInterface instead of ODataResponse
- IHttpProvider::send() method declares ResponseInterface as return type, but tests were mocking it to return ODataResponse
- ODataResponse does not implement ResponseInterface, causing type constraint violations in PHP 7.4+
- Updated createMockHttpProvider() to create proper PSR-7 response mocks with required methods
- This resolves all 19 test failures in CustomOptionsTest across PHP matrix (7.4, 8.0, 8.1, 8.2, 8.3, 8.4)

Co-authored-by: anderly &lt;573151+anderly@users.noreply.github.com&gt;

* Fix custom options test and concatenate method.

---------

Co-authored-by: copilot-swe-agent[bot] &lt;198982749+Copilot@users.noreply.github.com&gt;
Co-authored-by: anderly &lt;573151+anderly@users.noreply.github.com&gt;
Co-authored-by: Adam Anderly &lt;adam@anderly.com&gt;
diff --git a/README.md b/README.md
@@ -136,6 +136,59 @@ $airlines = $odataClient->from('Airlines')
     ->get();
 ```
 
+### Custom Query Options
+
+You can add custom query parameters to your OData requests that are not part of the standard OData specification. This is useful for passing additional parameters to your OData service:
+
+```php
+<?php
+
+use SaintSystems\OData\ODataClient;
+use SaintSystems\OData\GuzzleHttpProvider;
+
+$httpProvider = new GuzzleHttpProvider();
+$odataClient = new ODataClient($odataServiceUrl, null, $httpProvider);
+
+// Method 1: Add custom options using string format
+$people = $odataClient->from('People')
+    ->addOption('timeout=30')
+    ->addOption('format=minimal')
+    ->get();
+// Results in: /People?timeout=30&format=minimal
+
+// Method 2: Add custom options using array format  
+$people = $odataClient->from('People')
+    ->addOption(['timeout' => '30', 'debug' => 'true'])
+    ->get();
+// Results in: /People?timeout=30&debug=true
+
+// Method 3: Mix with standard OData parameters
+$people = $odataClient->from('People')
+    ->select('FirstName', 'LastName')
+    ->where('FirstName', 'Russell')
+    ->addOption('version=2.0')
+    ->get();
+// Results in: /People?$select=FirstName,LastName&$filter=FirstName eq 'Russell'&version=2.0
+
+// Method 4: Multiple addOption calls are merged (not overwritten)
+$people = $odataClient->from('People')
+    ->addOption('timeout=30')
+    ->addOption('format=minimal')
+    ->addOption(['debug' => 'true']);
+// Results in: /People?timeout=30&format=minimal&debug=true
+
+// Custom option keys are validated:
+// ✓ Valid: 'timeout', 'custom_param', 'kebab-case', 'camelCase'
+// ✗ Invalid: '$reserved' (starts with $), 'invalid key!' (special chars)
+```
+
+**Key Features:**
+- **Merging**: Multiple `addOption()` calls merge instead of overwriting
+- **Flexible**: Supports both string (`'key=value'`) and array (`['key' => 'value']`) formats
+- **Validated**: Custom option keys are validated to prevent conflicts with OData system parameters
+- **URL Encoded**: Special characters in keys and values are automatically URL encoded
+- **Fluent**: Chain with other query methods for clean, readable code
+
 ### Custom Timeout Configuration
 
 If you need to configure custom network timeouts for your OData requests, you can create a subclass of `ODataClient` and override the `createRequest` method:
diff --git a/src/Query/Builder.php b/src/Query/Builder.php
@@ -261,15 +261,124 @@ public function whereKey($id)
         
     /**
      * Add custom option to query parameters.
-     *
-     * @param string $options
+     * 
+     * This method merges custom query options instead of overwriting them.
+     * It supports both string and array formats:
+     * 
+     * String format: 'key=value'
+     * Array format: ['key' => 'value', 'key2' => 'value2']
+     * 
+     * Custom option keys must follow OData naming conventions:
+     * - Must not start with '$' or '@' (reserved for standard OData parameters)
+     * - Must be valid identifiers (alphanumeric and underscores)
+     * - Cannot be empty
+     * 
+     * Examples:
+     * $query->addOption('custom_param=value1')
+     *       ->addOption('another_param=value2');
+     * 
+     * $query->addOption(['timeout' => '30', 'format' => 'minimal']);
+     *
+     * @param string|array $option The custom option to add (string 'key=value' or associative array)
      *
      * @return $this
+     * @throws \InvalidArgumentException If option key is invalid
      */
     public function addOption($option)
     {
-        $this->customOption = $option;
+        if ($option === null || $option === '') {
+            return $this;
+        }
+
+        // Initialize customOption as array if not set
+        if (!isset($this->customOption)) {
+            $this->customOption = [];
+        }
+
+        // Convert existing string format to array for merging
+        if (is_string($this->customOption)) {
+            $this->customOption = $this->parseCustomOptionString($this->customOption);
+        }
+
+        // Convert current option to array format for processing
+        if (is_string($option)) {
+            $newOptions = $this->parseCustomOptionString($option);
+        } elseif (is_array($option)) {
+            $newOptions = $option;
+        } else {
+            throw new \InvalidArgumentException('Custom option must be a string or array');
+        }
+
+        // Validate and merge options
+        foreach ($newOptions as $key => $value) {
+            $this->validateCustomOptionKey($key);
+            $this->customOption[$key] = $value;
+        }
+
         return $this;
+    }
+
+    /**
+     * Parse a custom option string in 'key=value' format into an array.
+     *
+     * @param string $optionString The option string to parse
+     * @return array Parsed options as associative array
+     * @throws \InvalidArgumentException If string format is invalid
+     */
+    protected function parseCustomOptionString($optionString)
+    {
+        $options = [];
+        
+        if (strpos($optionString, '=') === false) {
+            throw new \InvalidArgumentException('Custom option string must contain "=" separator');
+        }
+
+        $pairs = explode(',', $optionString);
+        foreach ($pairs as $pair) {
+            $pair = trim($pair);
+            if (empty($pair)) {
+                continue;
+            }
+            
+            $parts = explode('=', $pair, 2);
+            if (count($parts) !== 2) {
+                throw new \InvalidArgumentException("Invalid custom option format: '$pair'. Expected 'key=value'");
+            }
+            
+            $key = trim($parts[0]);
+            $value = trim($parts[1]);
+            
+            if (empty($key)) {
+                throw new \InvalidArgumentException('Custom option key cannot be empty');
+            }
+            
+            $options[$key] = $value;
+        }
+        
+        return $options;
+    }
+
+    /**
+     * Validate a custom option key according to OData conventions.
+     *
+     * @param string $key The option key to validate
+     * @throws \InvalidArgumentException If key is invalid
+     */
+    protected function validateCustomOptionKey($key)
+    {
+        if (empty($key) || !is_string($key)) {
+            throw new \InvalidArgumentException('Custom option key must be a non-empty string');
+        }
+
+        // Check if key starts with '$' or '@' (reserved for OData system parameters)
+        if (strpos($key, '$') === 0 || strpos($key, '@') === 0) {
+            throw new \InvalidArgumentException("Custom option key '$key' cannot start with '\$' or '@' (reserved for OData system parameters)");
+        }
+
+        // Check for valid identifier pattern (alphanumeric, underscores, hyphens)
+        if (!preg_match('/^[a-zA-Z_][a-zA-Z0-9_-]*$/', $key)) {
+            throw new \InvalidArgumentException("Custom option key '$key' must be a valid identifier (alphanumeric, underscores, hyphens, starting with letter or underscore)");
+        }
     }    
 
     /**
diff --git a/src/Query/Grammar.php b/src/Query/Grammar.php
@@ -478,14 +478,14 @@ protected function compileOrdersToArray(Builder $query, $orders)
     /**
      * Compile the custom options portion of the query.
      *
-     * @param Builder $query
-     * @param string  $customOption
+     * @param Builder $query The query builder instance
+     * @param string|array|null $customOption The custom options to compile
      *
-     * @return string
+     * @return string The compiled custom options as query parameters
      */
     protected function compileCustomOption(Builder $query, $customOption)
     {
-        if (is_null($customOption)) {
+        if (is_null($customOption) || (is_array($customOption) && empty($customOption))) {
             return '';
         }
 
@@ -497,21 +497,26 @@ protected function compileCustomOption(Builder $query, $customOption)
     }
 
     /**
-     * Compile the composite Custom Options key portion of the query.
+     * Compile the composite Custom Options into a query parameter string.
      *
-     * @param Builder $query
-     * @param mixed   $customOption
+     * Converts an associative array of custom options into a 'key=value&key2=value2' format
+     * suitable for URL query parameters.
      *
-     * @return string
+     * @param array $customOption Associative array of custom options
+     *
+     * @return string Compiled custom options string
      */
     public function compileCompositeCustomOption($customOption)
     {
         $customOptions = [];
         foreach ($customOption as $key => $value) {
-            $customOptions[] = $key . '=' . $value;
+            // URL encode both key and value to handle special characters
+            $encodedKey = urlencode($key);
+            $encodedValue = urlencode($value);
+            $customOptions[] = $encodedKey . '=' . $encodedValue;
         }
 
-        return implode(',', $customOptions);
+        return implode('&', $customOptions);
     }    
 
     /**
@@ -590,15 +595,49 @@ public function columnize(array $properties)
      */
     protected function concatenate($segments)
     {
-        // return implode('', array_filter($segments, function ($value) {
-        //     return (string) $value !== '';
-        // }));
         $uri = '';
+        $queryParams = [];
+        $hasQueryString = false;
+        $hasEntitySet = false;
+        
         foreach ($segments as $segment => $value) {
             if ((string) $value !== '') {
-                $uri.= strpos($uri, '?$') ? '&' . $value : $value;
+                if ($segment === 'entitySet') {
+                    $hasEntitySet = true;
+                    $uri .= $value;
+                } else if ($segment === 'entityKey' || $segment === 'count') {
+                    // These are path segments, not query parameters
+                    $uri .= $value;
+                } else if ($segment === 'queryString') {
+                    // queryString already includes the '?' 
+                    $hasQueryString = true;
+                    // Skip it if empty or just '?'
+                    if ($value !== '?') {
+                        $uri .= $value;
+                    }
+                } else {
+                    // This is a query parameter - collect it
+                    $queryParams[] = $value;
+                }
             }
         }
+        
+        // Add query parameters if any
+        if (!empty($queryParams)) {
+            // Only add '?' if we have an entity set or already have content
+            if ($hasEntitySet || strlen($uri) > 0) {
+                // If we already have a queryString with '?', use '&' to join
+                if ($hasQueryString && strpos($uri, '?') !== false) {
+                    $uri .= '&' . implode('&', $queryParams);
+                } else {
+                    $uri .= '?' . implode('&', $queryParams);
+                }
+            } else {
+                // No entity set, just return the query params without '?'
+                $uri .= implode('&', $queryParams);
+            }
+        }
+        
         return $uri;
     }
 
diff --git a/tests/Query/CustomOptionsTest.php b/tests/Query/CustomOptionsTest.php
