From 4acfe9d1d84493ea90ba86d390f41ed1c01fffb6 Mon Sep 17 00:00:00 2001 From: Ed Wilde Date: Thu, 17 Oct 2024 10:17:12 +1300 Subject: [PATCH 1/2] FIX Make sure the `href` attribute is constructed correctly when trailing slash is used Prior to the fix, when trailing slash is enabled the url output is incorrect; the second test fails because the href output contains: `/api/v1/SilverStripe-ORM-DataObject/1/.xml`. --- src/DataFormatter/XMLDataFormatter.php | 4 +- tests/unit/XMLDataFormatterTest.php | 69 ++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/src/DataFormatter/XMLDataFormatter.php b/src/DataFormatter/XMLDataFormatter.php index c4ba879..5b8c559 100644 --- a/src/DataFormatter/XMLDataFormatter.php +++ b/src/DataFormatter/XMLDataFormatter.php @@ -117,9 +117,9 @@ class XMLDataFormatter extends DataFormatter { $className = $this->sanitiseClassName(get_class($obj)); $id = $obj->ID; - $objHref = Director::absoluteURL($this->config()->api_base . "$className/$obj->ID"); + $objHref = Director::absoluteURL($this->config()->api_base . "$className/$obj->ID" . ".xml"); - $xml = "<$className href=\"$objHref.xml\">\n"; + $xml = "<$className href=\"$objHref\">\n"; foreach ($this->getFieldsForObj($obj) as $fieldName => $fieldType) { // Field filtering if ($fields && !in_array($fieldName, $fields ?? [])) { diff --git a/tests/unit/XMLDataFormatterTest.php b/tests/unit/XMLDataFormatterTest.php index 8da70a1..328068a 100644 --- a/tests/unit/XMLDataFormatterTest.php +++ b/tests/unit/XMLDataFormatterTest.php @@ -5,6 +5,8 @@ namespace SilverStripe\RestfulServer\Tests; use SilverStripe\Dev\SapphireTest; use SilverStripe\RestfulServer\DataFormatter\XMLDataFormatter; use Exception; +use SilverStripe\Control\Controller; +use SilverStripe\ORM\DataObject; class XMLDataFormatterTest extends SapphireTest { @@ -74,4 +76,71 @@ XML $formatter = new XMLDataFormatter(); $formatter->convertStringToArray($inputXML); } + + /** + * Tests wrapper output of {@link XMLDataFormatter::convertDataObjectWithoutHeader()} + */ + public function testConvertDataObjectWithoutHeaderClassNameAttribute(): void + { + // Create a mock object + $mock = DataObject::create(); + $mock->ID = 1; + + // Disable trailing slash by default + Controller::config()->set('add_trailing_slash', false); + + // Create a formatter + $formatter = new XMLDataFormatter(); + + // Test the output + $expectedClass = 'SilverStripe-ORM-DataObject'; + $expectedHref = sprintf('http://localhost/api/v1/%s/%d.xml', $expectedClass, $mock->ID); + $expectedOutput = sprintf( + '<%s href="%s">%d', + $expectedClass, + $expectedHref, + $mock->ID, + $expectedClass + ); + + $actualOutput = $formatter->convertDataObjectWithoutHeader($mock); + + // remove line breaks and compare + $actualOutput = str_replace(["\n", "\r"], '', $actualOutput); + $this->assertEquals($expectedOutput, $actualOutput); + } + + /** + * Tests wrapper output of {@link XMLDataFormatter::convertDataObjectWithoutHeader()} when + * used with a forced trailing slash + */ + public function testConvertDataObjectWithoutHeaderClassNameAttributeWithTrailingSlash(): void + { + // Create a mock object + $mock = DataObject::create(); + $mock->ID = 1; + + // Enable trailing slash by default + Controller::config()->set('add_trailing_slash', true); + + // Create a formatter + $formatter = new XMLDataFormatter(); + + // Test the output + $expectedClass = 'SilverStripe-ORM-DataObject'; + $expectedHref = sprintf('http://localhost/api/v1/%s/%d.xml', $expectedClass, $mock->ID); + $expectedOutput = sprintf( + '<%s href="%s">%d', + $expectedClass, + $expectedHref, + $mock->ID, + $expectedClass + ); + + $actualOutput = $formatter->convertDataObjectWithoutHeader($mock); + + // remove line breaks and compare + $actualOutput = str_replace(["\n", "\r"], '', $actualOutput); + $this->assertEquals($expectedOutput, $actualOutput); + } } From 83cbb59c722dc91f4fa01df46c8b89a9957ed302 Mon Sep 17 00:00:00 2001 From: Ed Wilde Date: Thu, 17 Oct 2024 12:04:03 +1300 Subject: [PATCH 2/2] FIX Apply the same fix to other usage of `absoluteURL()` --- src/DataFormatter/XMLDataFormatter.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/DataFormatter/XMLDataFormatter.php b/src/DataFormatter/XMLDataFormatter.php index 5b8c559..db91c34 100644 --- a/src/DataFormatter/XMLDataFormatter.php +++ b/src/DataFormatter/XMLDataFormatter.php @@ -160,11 +160,11 @@ class XMLDataFormatter extends DataFormatter $fieldName = $relName . 'ID'; if ($obj->$fieldName) { - $href = Director::absoluteURL($this->config()->api_base . "$relClass/" . $obj->$fieldName); + $href = Director::absoluteURL($this->config()->api_base . "$relClass/" . $obj->$fieldName . ".xml"); } else { - $href = Director::absoluteURL($this->config()->api_base . "$className/$id/$relName"); + $href = Director::absoluteURL($this->config()->api_base . "$className/$id/$relName" . ".xml"); } - $xml .= "<$relName linktype=\"has_one\" href=\"$href.xml\" id=\"" . $obj->$fieldName + $xml .= "<$relName linktype=\"has_one\" href=\"$href\" id=\"" . $obj->$fieldName . "\">\n"; } @@ -190,8 +190,8 @@ class XMLDataFormatter extends DataFormatter $items = $obj->$relName(); if ($items) { foreach ($items as $item) { - $href = Director::absoluteURL($this->config()->api_base . "$relClass/$item->ID"); - $xml .= "<$relClass href=\"$href.xml\" id=\"{$item->ID}\">\n"; + $href = Director::absoluteURL($this->config()->api_base . "$relClass/$item->ID" . ".xml"); + $xml .= "<$relClass href=\"$href\" id=\"{$item->ID}\">\n"; } } $xml .= "\n"; @@ -221,8 +221,8 @@ class XMLDataFormatter extends DataFormatter $items = $obj->$relName(); if ($items) { foreach ($items as $item) { - $href = Director::absoluteURL($this->config()->api_base . "$relClass/$item->ID"); - $xml .= "<$relClass href=\"$href.xml\" id=\"{$item->ID}\">\n"; + $href = Director::absoluteURL($this->config()->api_base . "$relClass/$item->ID" . ".xml"); + $xml .= "<$relClass href=\"$href\" id=\"{$item->ID}\">\n"; } } $xml .= "\n";