Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions src/IODataEntityResponse.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

namespace SaintSystems\OData;

Comment on lines +2 to +4
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing file-level and interface docblocks. The interface should include documentation describing its purpose and relationship to IODataResponse, following the codebase convention (e.g., as seen in other interface files).

Suggested change
namespace SaintSystems\OData;
/**
* IODataEntityResponse interface file.
*
* Defines the contract for OData entity response objects, which represent
* responses containing single entities or collections of entities from an OData service.
* Extends IODataResponse to provide additional methods for entity-specific
* operations such as object conversion, skip token retrieval, and ID access.
*
* @package SaintSystems\OData
*/
namespace SaintSystems\OData;
/**
* Interface for OData entity response objects.
*
* Extends IODataResponse to provide methods for converting the response
* to SDK objects, retrieving skip tokens for pagination, and accessing
* the entity ID after insert operations. Used by ODataClient to represent
* responses containing entities or entity sets from an OData service.
*
* @see IODataResponse
*/

Copilot uses AI. Check for mistakes.
interface IODataEntityResponse extends IODataResponse
{
/**
* Converts the response JSON object to a OData SDK object
*
* @param mixed $returnType The type to convert the object(s) to
*
* @return mixed object or array of objects of type $returnType
*/
public function getResponseAsObject($returnType);

/**
* Gets the skip token of a response object from OData
*
* @return string skip token, if provided
*/
public function getSkipToken();

/**
* Gets the Id of response object (if set) from OData
*
* @return mixed id if this was an insert, if provided
*/
public function getId();
}
23 changes: 0 additions & 23 deletions src/IODataResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,27 +31,4 @@ public function getStatus();
* @var array The response headers
*/
public function getHeaders();

/**
* Converts the response JSON object to a OData SDK object
*
* @param mixed $returnType The type to convert the object(s) to
*
* @return mixed object or array of objects of type $returnType
*/
public function getResponseAsObject($returnType);

/**
* Gets the skip token of a response object from OData
*
* @return string skip token, if provided
*/
public function getSkipToken();

/**
* Gets the Id of response object (if set) from OData
*
* @return mixed id if this was an insert, if provided
*/
public function getId();
}
219 changes: 219 additions & 0 deletions src/ODataBatchResponse.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
<?php

namespace SaintSystems\OData;

class ODataBatchResponse implements IODataResponse
{
Comment on lines +5 to +6
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing class-level docblock documentation. The class should include a docblock describing its purpose, usage example, and parameters for the constructor, following the existing codebase convention seen in ODataResponse.php (lines 21-27).

Suggested change
class ODataBatchResponse implements IODataResponse
{
/**
* Represents a response to an OData batch request.
*
* This class parses the multipart/mixed batch response from an OData service,
* extracting individual responses and changesets. It provides access to the
* original request, response body, headers, HTTP status code, and parsed batch responses.
*
* Usage example:
* <code>
* $batchResponse = new ODataBatchResponse($request, $body, $httpStatusCode, $headers);
* $responses = $batchResponse->getResponses();
* </code>
*
* @param IODataRequest $request The original batch request object.
* @param string|null $body The raw response body from the batch request.
* @param string|null $httpStatusCode The HTTP status code of the batch response.
* @param array $headers The response headers.
*/
class ODataBatchResponse implements IODataResponse

Copilot uses AI. Check for mistakes.
private IODataRequest $request;
private ?string $body;
private array $headers;
private ?string $httpStatusCode;
private array $responses;
private ?string $boundary;

public function __construct(IODataRequest $request, ?string $body = null, ?string $httpStatusCode = null, array $headers = array())
{
$this->request = $request;
$this->body = $body;
$this->httpStatusCode = $httpStatusCode;
$this->headers = $headers;
$this->boundary = $this->extractBoundary();
$this->responses = $this->parseBatchResponse();
}

private function extractBoundary(): ?string
{
$contentType = $this->getContentTypeHeader();
if ($contentType !== null && $contentType !== '' && preg_match('/boundary=(["\']?)([^"\';]+)\1/', $contentType, $matches)) {
$boundary = $matches[2];

if (strpos(strtolower($boundary), 'batchresponse') !== false) {
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The boundary validation in extractBoundary() is inconsistent with the factory detection logic. The factory checks for 'boundary=batchresponse_' while this method checks if the boundary contains 'batchresponse' anywhere (case-insensitive). This could lead to false positives where boundaries containing 'batchresponse' in the middle or end would be accepted. Consider aligning the validation logic between the factory and this class.

Suggested change
if (strpos(strtolower($boundary), 'batchresponse') !== false) {
if (stripos($boundary, 'batchresponse_') === 0) {

Copilot uses AI. Check for mistakes.
return $boundary;
Comment on lines +27 to +31
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex pattern /boundary=(["\']?)([^"\';]+)\1/ doesn't handle boundaries that might contain special regex characters or have spaces. Additionally, the pattern [^"\';]+ will stop at semicolons, which means if the boundary value is followed by additional parameters (e.g., boundary=xyz; charset=utf-8), it will work, but boundary values containing semicolons would fail. While semicolons in boundaries are rare, consider using a more robust parsing approach or document this limitation.

Suggested change
if ($contentType !== null && $contentType !== '' && preg_match('/boundary=(["\']?)([^"\';]+)\1/', $contentType, $matches)) {
$boundary = $matches[2];
if (strpos(strtolower($boundary), 'batchresponse') !== false) {
return $boundary;
if ($contentType !== null && $contentType !== '') {
// Improved regex: matches boundary=foo, boundary="foo;bar", boundary='foo bar', etc.
if (preg_match('/boundary\s*=\s*(?:"([^"]+)"|\'([^\']+)\'|([^\s;]+))/i', $contentType, $matches)) {
// $matches[1]: double-quoted, $matches[2]: single-quoted, $matches[3]: unquoted
$boundary = $matches[1] ?? $matches[2] ?? $matches[3] ?? null;
if ($boundary !== null && strpos(strtolower($boundary), 'batchresponse') !== false) {
return $boundary;
}

Copilot uses AI. Check for mistakes.
}
}
return null;
}

private function getContentTypeHeader(): ?string
{
foreach ($this->headers as $key => $value) {
if (strtolower($key) === 'content-type') {
return is_array($value) ? $value[0] : $value;
}
}
return null;
}

private function parseBatchResponse(): array
{
if ($this->body === null || $this->body === '' || $this->boundary === null || $this->boundary === '') {
return array();
}

$responses = array();
$parts = explode('--' . $this->boundary, $this->body);

foreach ($parts as $part) {
$part = trim($part);
// Skip empty parts and boundary end marker
if ($part === '' || $part === '--' || $part === "\r\n--" || trim($part, "\r\n-") === '') {
continue;
}

if ($this->isChangesetPart($part)) {
$changesetResponses = $this->parseChangesetPart($part);
$responses = array_merge($responses, $changesetResponses);
} else {
$response = $this->parseIndividualResponse($part);
if ($response !== null) {
$responses[] = $response;
}
}
}

return $responses;
}

private function isChangesetPart(string $part): bool
{
return preg_match('/Content-Type:\s*multipart\/mixed;\s*boundary=["\']?[^"\'\s]*changeset[^"\'\s]*["\']?/i', $part) === 1;
}

private function parseChangesetPart(string $part): array
{
if (preg_match('/boundary=(["\']?)([^"\'\r\n;]+)\1/i', $part, $matches) !== 1) {
return array();
}

$changesetBoundary = $matches[2];

if ($changesetBoundary === '' || strpos(strtolower($changesetBoundary), 'changeset') === false) {
return array();
}

// Find where changeset content starts (after empty line)
$changesetContent = $this->extractChangesetContent($part);

// Parse individual responses within changeset
$changesetParts = explode('--' . $changesetBoundary, $changesetContent);
$responses = array();

foreach ($changesetParts as $changesetPart) {
$changesetPart = trim($changesetPart);
if ($changesetPart === '' || $changesetPart === '--' || trim($changesetPart, "\r\n-") === '') {
continue;
}

$response = $this->parseIndividualResponse($changesetPart);
if ($response !== null) {
$responses[] = $response;
}
}

return $responses;
}

private function extractChangesetContent(string $part): string
{
$lines = explode("\n", $part);
$contentStarted = false;
$content = array();

foreach ($lines as $line) {
$line = rtrim($line, "\r");

// Skip until we find an empty line (end of changeset headers)
if (!$contentStarted) {
if (trim($line) === '') {
$contentStarted = true;
}
continue;
}

$content[] = $line;
}

return implode("\n", $content);
Comment on lines +118 to +136
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extractChangesetContent() method splits the entire part into lines using explode("\n"), which creates an array of all lines in memory. For large batch responses with significant changeset content, this could be memory-intensive. Consider using a more memory-efficient approach if supporting large payloads is a requirement, such as using strpos() to find the first empty line and then using substr() to extract the content.

Suggested change
$lines = explode("\n", $part);
$contentStarted = false;
$content = array();
foreach ($lines as $line) {
$line = rtrim($line, "\r");
// Skip until we find an empty line (end of changeset headers)
if (!$contentStarted) {
if (trim($line) === '') {
$contentStarted = true;
}
continue;
}
$content[] = $line;
}
return implode("\n", $content);
// Find the position of the first empty line (end of headers)
$pos = false;
// Try CRLF first
$pos = strpos($part, "\r\n\r\n");
$delimiterLength = 4;
if ($pos === false) {
// Try LF only
$pos = strpos($part, "\n\n");
$delimiterLength = 2;
}
if ($pos === false) {
// No header/content delimiter found, return empty string
return '';
}
// Extract everything after the header/content delimiter
return substr($part, $pos + $delimiterLength);

Copilot uses AI. Check for mistakes.
}

private function parseIndividualResponse(string $part): ?ODataResponse
{
$lines = explode("\n", $part);
$inHeaders = true;
$responseHeaders = array();
$responseBody = '';
$statusCode = null;
$foundHttpResponse = false;

foreach ($lines as $line) {
$line = rtrim($line, "\r");

if ($inHeaders) {
if (trim($line) === '') {
// Only switch to body parsing if we've found an HTTP response line
if ($foundHttpResponse) {
$inHeaders = false;
}
continue;
}

if (strpos($line, 'HTTP/') === 0) {
$statusParts = explode(' ', $line, 3);
$statusCode = (array_key_exists(1, $statusParts) && $statusParts[1] !== null) ? $statusParts[1] : (string)HttpStatusCode::OK;
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type mismatch in status code handling. HttpStatusCode::OK is an integer constant (200), but the function expects to return a string type. The cast should be (string) of the integer value 200, not the constant name. Consider: $statusCode = isset($statusParts[1]) ? $statusParts[1] : '200';

Suggested change
$statusCode = (array_key_exists(1, $statusParts) && $statusParts[1] !== null) ? $statusParts[1] : (string)HttpStatusCode::OK;
$statusCode = (array_key_exists(1, $statusParts) && $statusParts[1] !== null) ? $statusParts[1] : '200';

Copilot uses AI. Check for mistakes.
$foundHttpResponse = true;
continue;
}

// Only parse headers after we've found the HTTP response line
if ($foundHttpResponse && strpos($line, ':') !== false) {
list($key, $value) = explode(':', $line, 2);
$responseHeaders[trim($key)] = trim($value);
}
} else {
$responseBody .= $line . "\n";
}
}
Comment on lines +141 to +175
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parseIndividualResponse() method splits the entire part into lines using explode("\n"). For batch responses with many operations or large response bodies, this creates unnecessary line arrays in memory. Consider using a string position-based approach or stream-based parsing for better memory efficiency with large responses.

Copilot uses AI. Check for mistakes.

$responseBody = trim($responseBody);
Comment on lines +141 to +177
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concatenating lines with "\n" may not preserve the original line endings from the response. If the original response uses "\r\n" (CRLF), the reconstructed body will have inconsistent line endings. While line 149 strips "\r", the body reconstruction only uses "\n". This could cause issues with signature verification or content-length validation. Consider preserving the original line endings or consistently normalizing them.

Suggested change
$lines = explode("\n", $part);
$inHeaders = true;
$responseHeaders = array();
$responseBody = '';
$statusCode = null;
$foundHttpResponse = false;
foreach ($lines as $line) {
$line = rtrim($line, "\r");
if ($inHeaders) {
if (trim($line) === '') {
// Only switch to body parsing if we've found an HTTP response line
if ($foundHttpResponse) {
$inHeaders = false;
}
continue;
}
if (strpos($line, 'HTTP/') === 0) {
$statusParts = explode(' ', $line, 3);
$statusCode = (array_key_exists(1, $statusParts) && $statusParts[1] !== null) ? $statusParts[1] : (string)HttpStatusCode::OK;
$foundHttpResponse = true;
continue;
}
// Only parse headers after we've found the HTTP response line
if ($foundHttpResponse && strpos($line, ':') !== false) {
list($key, $value) = explode(':', $line, 2);
$responseHeaders[trim($key)] = trim($value);
}
} else {
$responseBody .= $line . "\n";
}
}
$responseBody = trim($responseBody);
// Split into lines and capture line endings
$parts = preg_split('/(\r\n|\n)/', $part, -1, PREG_SPLIT_DELIM_CAPTURE);
$inHeaders = true;
$responseHeaders = array();
$responseBody = '';
$statusCode = null;
$foundHttpResponse = false;
// $parts is an array alternating between line content and line ending
$numParts = count($parts);
for ($i = 0; $i < $numParts; $i += 2) {
$line = isset($parts[$i]) ? $parts[$i] : '';
$lineEnding = isset($parts[$i + 1]) ? $parts[$i + 1] : '';
// Remove only trailing \r if present (for header parsing)
$headerLine = rtrim($line, "\r");
if ($inHeaders) {
if (trim($headerLine) === '') {
// Only switch to body parsing if we've found an HTTP response line
if ($foundHttpResponse) {
$inHeaders = false;
}
continue;
}
if (strpos($headerLine, 'HTTP/') === 0) {
$statusParts = explode(' ', $headerLine, 3);
$statusCode = (array_key_exists(1, $statusParts) && $statusParts[1] !== null) ? $statusParts[1] : (string)HttpStatusCode::OK;
$foundHttpResponse = true;
continue;
}
// Only parse headers after we've found the HTTP response line
if ($foundHttpResponse && strpos($headerLine, ':') !== false) {
list($key, $value) = explode(':', $headerLine, 2);
$responseHeaders[trim($key)] = trim($value);
}
} else {
// Preserve original line endings in body
$responseBody .= $line . $lineEnding;
}
}
// Optionally trim only leading/trailing whitespace, not line endings
$responseBody = trim($responseBody, "\r\n\t ");

Copilot uses AI. Check for mistakes.

if ($statusCode !== null) {
return new ODataResponse($this->request, $responseBody, $statusCode, $responseHeaders);
}

return null;
}

public function getBody(): array
{
$bodies = array();
foreach ($this->responses as $response) {
$bodies[] = $response->getBody();
}
return $bodies;
}
Comment on lines +186 to +193
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing method docblock. Public method getBody() should include documentation describing that it returns an array of decoded bodies from all individual responses, following the documentation pattern used in the codebase (e.g., ODataResponse.php lines 102-106).

Copilot uses AI. Check for mistakes.

public function getRawBody(): ?string
{
return $this->body;
}
Comment on lines +195 to +198
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing method docblock. Public method getRawBody() should include documentation, following the pattern in ODataResponse.php (lines 112-120).

Copilot uses AI. Check for mistakes.

public function getStatus(): ?string
{
return $this->httpStatusCode;
}
Comment on lines +200 to +203
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing method docblock. Public method getStatus() should include documentation, following the pattern in ODataResponse.php (lines 122-130).

Copilot uses AI. Check for mistakes.

public function getHeaders(): array
{
return $this->headers;
}
Comment on lines +205 to +208
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing method docblock. Public method getHeaders() should include documentation, following the pattern in ODataResponse.php (lines 132-140).

Copilot uses AI. Check for mistakes.

public function getResponses(): array
{
return $this->responses;
}
Comment on lines +210 to +213
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing method docblock. Public method getResponses() should include documentation describing that it returns an array of ODataResponse objects parsed from the batch response.

Copilot uses AI. Check for mistakes.

public function getResponse(int $index): ?ODataResponse
{
return (array_key_exists($index, $this->responses) && $this->responses[$index] !== null) ? $this->responses[$index] : null;
}
Comment on lines +215 to +218
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing method docblock. Public method getResponse() should include documentation describing the @param and @return values.

Copilot uses AI. Check for mistakes.
}
4 changes: 2 additions & 2 deletions src/ODataRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,9 @@ public function execute()
return [(string) $result->getBody(), null];
}

// Wrap response in ODataResponse layer
// Wrap response in appropriate ODataResponse layer using factory
try {
$response = new ODataResponse(
$response = ODataResponseFactory::create(
$this,
(string) $result->getBody(),
$result->getStatusCode(),
Expand Down
2 changes: 1 addition & 1 deletion src/ODataResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
* @package SaintSystems.OData
* @license https://opensource.org/licenses/MIT MIT License
*/
class ODataResponse implements IODataResponse
class ODataResponse implements IODataEntityResponse
{
/**
* The request
Expand Down
31 changes: 31 additions & 0 deletions src/ODataResponseFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

namespace SaintSystems\OData;

class ODataResponseFactory
{
public static function create(IODataRequest $request, ?string $body = null, ?string $statusCode = null, array $headers = array())
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return type of create() should be declared as IODataResponse to ensure type safety and consistency with the interface hierarchy. Currently, the method returns either ODataBatchResponse or ODataResponse, both of which implement IODataResponse. Adding the return type hint would make the API contract clearer: public static function create(...): IODataResponse

Suggested change
public static function create(IODataRequest $request, ?string $body = null, ?string $statusCode = null, array $headers = array())
public static function create(IODataRequest $request, ?string $body = null, ?string $statusCode = null, array $headers = array()): IODataResponse

Copilot uses AI. Check for mistakes.
{
$contentType = self::getContentType($headers);

// Batch response detection - Microsoft uses multipart/mixed with batchresponse_ boundary
if ($contentType !== null &&
strpos($contentType, 'multipart/mixed') === 0 &&
strpos($contentType, 'boundary=batchresponse_') !== false) {
Comment on lines +13 to +14
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The batch response detection logic is overly strict by checking for 'boundary=batchresponse_' specifically. According to the OData specification, any boundary string is valid as long as Content-Type is 'multipart/mixed'. The current implementation would fail for batch responses with custom boundary names that don't start with 'batchresponse_'. Consider checking only for 'multipart/mixed' content type and then letting the ODataBatchResponse class validate the boundary pattern internally.

Suggested change
strpos($contentType, 'multipart/mixed') === 0 &&
strpos($contentType, 'boundary=batchresponse_') !== false) {
strpos($contentType, 'multipart/mixed') === 0) {

Copilot uses AI. Check for mistakes.
return new ODataBatchResponse($request, $body, $statusCode, $headers);
}

return new ODataResponse($request, $body, $statusCode, $headers);
}
Comment on lines +1 to +19
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing class-level and method docblocks. The factory class and its methods should include documentation describing their purpose, parameters, and return types, following the existing codebase convention seen in ODataResponse.php.

Copilot uses AI. Check for mistakes.

private static function getContentType(array $headers): ?string
{
foreach ($headers as $key => $value) {
if (strtolower($key) === 'content-type' && $value !== null) {
return $value;
}
}

return null;
}
}
Loading
Loading