Merge remote-tracking branch 'upstream/dev' into b/xml-serialization

README.md

@@ -17,7 +17,7 @@ The Sitemap provides a mechanism for displaying Sitemap style information (the u
 
 ## Installation
 
-```bash
+```shell
 $ composer require loadsys/cakephp_sitemap
 ```
 
@@ -29,8 +29,8 @@ Plugin::load('Sitemap', ['bootstrap' => false, 'routes' => true]);
 
 OR
 
-```php
-bin/cake plugin load Sitemap
+```shell
+$ bin/cake plugin load Sitemap
 ```
 
 ## Usage
@@ -71,16 +71,16 @@ $this->addBehavior('Sitemap.Sitemap');
 ],
 ```
 
-* To modify these options for instance to change the `changefreq` when listing records, updated the `addBehavior` method call for the `Table` in question like so:
+* To modify these options for instance to change the `changefreq` when listing records, update the `addBehavior` method call for the `Table` in question like so:
 
 ```php
 $this->addBehavior('Sitemap.Sitemap', ['changefreq' => 'weekly']);
 ```
 
 * To customize the url generated for each record create a method named `getUrl` in the matching `Table` class.
 
-```
-public function getUrl(\App\Model\Entity $entity) {
+```php
+public function getUrl(\Cake\ORM\Entity $entity) {
 	return \Cake\Routing\Router::url(
 		[
 			'prefix' => false,
@@ -94,6 +94,8 @@ public function getUrl(\App\Model\Entity $entity) {
 }
 ```
 
+* To customize the templates used when displaying the Sitemap, the CakePHP Book provides information regarding [overriding Plugin Templates](http://book.cakephp.org/3.0/en/plugins.html#overriding-plugin-templates-from-inside-your-application).
+
 ## Contributing
 
 ### Code of Conduct

composer.json

@@ -23,8 +23,7 @@
   "require": {
     "php": ">=5.6.0",
     "cakephp/cakephp": "~3.1",
-    "composer/installers": "~1.0",
-    "composer-plugin-api": "~1.0"
+    "composer/installers": "~1.0"
   },
   "require-dev": {
     "phpunit/phpunit": "~4.8",

src/Lib/Iterators/ExtFilteredDirIterator.php

@@ -55,6 +55,7 @@ public function __construct($path, $allowedExtensions = null) {
 	 */
 	public function accept() {
 		$current = parent::current();
+
 		return (
 			!$current->isDot()
 			&&

src/Lib/Iterators/PagesIterator.php

@@ -79,6 +79,7 @@ public function __construct($path, $depth, $webroot, $allowedExtensions = null)
 	 */
 	public function accept() {
 		$current = parent::current();
+
 		return parent::accept() && (strpos($current->getBasename('.ctp'), 'admin_') !== 0);
 	}
 

src/Model/Behavior/SitemapBehavior.php

@@ -20,7 +20,9 @@ class SitemapBehavior extends Behavior {
 	 *
 	 * @var array
 	 */
+	// @codingStandardsIgnoreStart
 	protected $_defaultConfig = [
+	// @codingStandardsIgnoreEnd
 		'cacheConfigKey' => 'default',
 		'lastmod' => 'modified',
 		'changefreq' => 'daily',
@@ -64,6 +66,7 @@ public function initialize(array $config) {
 	/**
 	 * Return the URL for the primary view action for an Entity.
 	 *
+	 * @param \Cake\ORM\Entity $entity Entity object passed in to return the url for.
 	 * @return string Returns the URL string.
 	 */
 	public function returnUrlForEntity(Entity $entity) {
@@ -82,12 +85,12 @@ public function returnUrlForEntity(Entity $entity) {
 	/**
 	 * Find the Sitemap Records for a Table.
 	 *
-	 * @param Query $query The Query being modified.
+	 * @param \Cake\ORM\Query $query The Query being modified.
 	 * @param array $options The array of options for the find.
-	 * @return Query Returns the modified Query object.
+	 * @return \Cake\ORM\Query Returns the modified Query object.
 	 */
 	public function findSitemapRecords(Query $query, array $options) {
-		$query
+		$query = $query
 			->where($this->_config['conditions'])
 			->cache("sitemap_{$query->repository()->alias()}", $this->_config['cacheConfigKey'])
 			->order($this->_config['order'])
@@ -96,7 +99,7 @@ public function findSitemapRecords(Query $query, array $options) {
 			});
 
 		if (!empty($this->_config['fields'])) {
-			$query->select($this->_config['fields']);
+			$query = $query->select($this->_config['fields']);
 		}
 
 		return $query;
@@ -120,14 +123,15 @@ public function mapResults(ResultSetInterface $results) {
 	/**
 	 * Modify an entity with new `_` fields for the Sitemap display.
 	 *
-	 * @param \Cake\Orm\Entity $entity The entity being modified.
-	 * @return Entity
+	 * @param \Cake\ORM\Entity $entity The entity being modified.
+	 * @return \Cake\ORM\Entity Returns the modified entity.
 	 */
 	public function mapEntity(Entity $entity) {
 		$entity['_loc'] = $this->_table->getUrl($entity);
 		$entity['_lastmod'] = $entity->{$this->_config['lastmod']};
 		$entity['_changefreq'] = $this->_config['changefreq'];
 		$entity['_priority'] = $this->_config['priority'];
+
 		return $entity;
 	}
 }

tests/TestCase/Model/Behavior/SitemapBehaviorTest.php

@@ -1,6 +1,6 @@
 <?php
 /**
- * Tests for the Sitemap Behavior class.
+ * Tests for the SitemapBehavior class.
  */
 namespace Sitemap\Test\TestCase\Model\Behavior;
 
@@ -14,14 +14,20 @@
 
 /**
  * \Siteamp\Test\TestCase\Model\Behavior\TestSitemapBehavior
+ *
+ * Class to expose the protected properties and methods for SitemapBehavior.
  */
 class TestSitemapBehavior extends SitemapBehavior {
+	// @codingStandardsIgnoreStart
 	public $_config;
 	public $_table;
+	// @codingStandardsIgnoreEnd
 }
 
 /**
  * \Siteamp\Test\TestCase\Model\Behavior\SitemapBehaviorTest
+ *
+ * Test class for the SitemapBehavior class.
  */
 class SitemapBehaviorTest extends TestCase {
 	/**
@@ -32,6 +38,7 @@ class SitemapBehaviorTest extends TestCase {
 	public $fixtures = [
 		'plugin.sitemap.pages',
 	];
+
 	/**
 	 * setUp method
 	 *
@@ -133,62 +140,162 @@ public function testReturnUrlForEntity() {
 	}
 
 	/**
-	 * Test findSitemapRecords method
+	 * Test ::findSitemapRecords when the Fields config is set.
 	 *
-	 * @param array $sitemapConfig The Configs for the Sitemap Behavior.
-	 * @param int $expectedCount The expected number of returning results from the find.
 	 * @return void
-	 * @dataProvider providerFindSitemapRecords
 	 */
-	public function testFindSitemapRecords($sitemapConfig, $expectedCount) {
-		$this->Pages->removeBehavior('Sitemap');
-		$this->Pages->addBehavior('Sitemap.Sitemap', $sitemapConfig);
+	public function testFindSitemapRecordsFields() {
+		$configs = [
+			'conditions' => [
+				'field1' => 'value1',
+			],
+			'cacheConfigKey' => 'cache-key-asdf',
+			'order' => [
+				'field2' => 'value2',
+			],
+			'fields' => [
+				'field3' => 'value3',
+			],
+		];
+		$options = [
+			'foo' => 'baz',
+		];
+
+		$TableMock = $this
+			->getMockBuilder('\Cake\ORM\Table')
+			->disableOriginalConstructor()
+			->getMock();
+		$SitemapBehavior = $this
+			->getMockBuilder('\Sitemap\Model\Behavior\SitemapBehavior')
+			->setMethods(['mapResults'])
+			->setConstructorArgs([$TableMock, $configs])
+			->getMock();
+
+		$QueryMock = $this->getMockBuilder('\Cake\ORM\Query')
+			->setMethods([
+				'where',
+				'cache',
+				'order',
+				'formatResults',
+				'select',
+				'repository',
+				'alias',
+			])
+			->disableOriginalConstructor()
+			->getMock();
+
+		$QueryMock->expects($this->once())
+			->method('where')
+			->with($configs['conditions'])
+			->will($this->returnSelf());
+		$QueryMock->expects($this->once())
+			->method('repository')
+			->with()
+			->will($this->returnSelf());
+		$QueryMock->expects($this->once())
+			->method('alias')
+			->with()
+			->will($this->returnValue('alias-canary'));
+		$QueryMock->expects($this->once())
+			->method('cache')
+			->with('sitemap_alias-canary', $configs['cacheConfigKey'])
+			->will($this->returnSelf());
+		$QueryMock->expects($this->once())
+			->method('order')
+			->with($configs['order'])
+			->will($this->returnSelf());
+		$QueryMock->expects($this->once())
+			->method('formatResults')
+			->with($this->isInstanceOf('closure'))
+			->will($this->returnSelf());
+		$QueryMock->expects($this->once())
+			->method('select')
+			->with($configs['fields'])
+			->will($this->returnValue('canary'));
 
-		$query = $this->Pages->find('forSitemap');
-		$count = $query->count();
+		$output = $SitemapBehavior->findSitemapRecords($QueryMock, $options);
 		$this->assertEquals(
-			$expectedCount,
-			$count,
-			"The count of the pages should be equal to {$expectedCount}"
+			'canary',
+			$output,
+			'The output from ::findSitemapRecords should be our mocked response from the Query Object.'
 		);
 	}
 
 	/**
-	 * DataProvider for testFindSitemapRecords.
+	 * Test ::findSitemapRecords when the Fields config is not set.
 	 *
-	 * @return array Data inputs for testFindSitemapRecords.
+	 * @return void
 	 */
-	public function providerFindSitemapRecords() {
-		return [
-			'Empty Config Options' => [
-				[],
-				5,
+	public function testFindSitemapRecordsNoFields() {
+		$configs = [
+			'conditions' => [
+				'field1' => 'value1',
 			],
-			'Non Conditional Config Options' => [
-				[
-					'lastmod' => 'modified_date',
-				],
-				5,
+			'cacheConfigKey' => 'cache-key-asdf',
+			'order' => [
+				'field2' => 'value2',
 			],
+		];
+		$options = [
+			'foo' => 'baz',
+		];
 
-			'Is Indexed TRUE Config Options' => [
-				[
-					'conditions' => [
-						'is_indexed' => true,
-					],
-				],
-				4,
-			],
+		$TableMock = $this
+			->getMockBuilder('\Cake\ORM\Table')
+			->disableOriginalConstructor()
+			->getMock();
+		$SitemapBehavior = $this
+			->getMockBuilder('\Sitemap\Model\Behavior\SitemapBehavior')
+			->setMethods(['mapResults'])
+			->setConstructorArgs([$TableMock, $configs])
+			->getMock();
 
-			'Is Indexed FALSE Config Options' => [
-				[
-					'conditions' => [
-						'is_indexed' => false,
-					],
-				],
-				1,
-			],
-		];
+		$QueryMock = $this->getMockBuilder('\Cake\ORM\Query')
+			->setMethods([
+				'where',
+				'cache',
+				'order',
+				'formatResults',
+				'select',
+				'repository',
+				'alias',
+			])
+			->disableOriginalConstructor()
+			->getMock();
+
+		$QueryMock->expects($this->once())
+			->method('where')
+			->with($configs['conditions'])
+			->will($this->returnSelf());
+		$QueryMock->expects($this->once())
+			->method('repository')
+			->with()
+			->will($this->returnSelf());
+		$QueryMock->expects($this->once())
+			->method('alias')
+			->with()
+			->will($this->returnValue('alias-canary'));
+		$QueryMock->expects($this->once())
+			->method('cache')
+			->with('sitemap_alias-canary', $configs['cacheConfigKey'])
+			->will($this->returnSelf());
+		$QueryMock->expects($this->once())
+			->method('order')
+			->with($configs['order'])
+			->will($this->returnSelf());
+		$QueryMock->expects($this->once())
+			->method('formatResults')
+			->with($this->isInstanceOf('closure'))
+			->will($this->returnValue('canary'));
+		$QueryMock->expects($this->never())
+			->method('select');
+
+		$output = $SitemapBehavior->findSitemapRecords($QueryMock, $options);
+		$this->assertEquals(
+			'canary',
+			$output,
+			'The output from ::findSitemapRecords should be our mocked response from the Query Object.'
+		);
 	}
 
 	/**