From e1efbe50d8486cc6483e8522474fcf45f8c5088a Mon Sep 17 00:00:00 2001 From: Bilge Date: Sat, 15 Oct 2016 00:50:43 +0100 Subject: [PATCH 1/3] Added EncapsulatedOptions::merge() and accompanying test. --- .travis.yml | 1 + composer.json | 2 +- src/Options/EncapsulatedOptions.php | 36 +++++++++++++++++ src/Options/MergeException.php | 10 +++++ test/Porter/Options/TestOptions.php | 2 +- .../Options/EncapsulatedOptionsTest.php | 39 +++++++++++++++++++ 6 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 src/Options/MergeException.php diff --git a/.travis.yml b/.travis.yml index 5575707..06d2664 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,6 +9,7 @@ php: - 5.5 - 5.6 - 7.0 + - 7.1 install: - alias composer=composer\ -n && composer selfupdate diff --git a/composer.json b/composer.json index cc0fb25..c271665 100644 --- a/composer.json +++ b/composer.json @@ -10,7 +10,7 @@ "license": "LGPL-3.0", "require": { "php": ">=5.5", - "scriptfusion/mapper": "^1|^0", + "scriptfusion/mapper": "^1", "scriptfusion/static-class": "^1", "scriptfusion/retry-error-handlers": "^1", "psr/cache": "^1", diff --git a/src/Options/EncapsulatedOptions.php b/src/Options/EncapsulatedOptions.php index 03dc621..b4e6ade 100644 --- a/src/Options/EncapsulatedOptions.php +++ b/src/Options/EncapsulatedOptions.php @@ -20,6 +20,42 @@ final public function copy() return $this->options + $this->defaults; } + /** + * Merges the specified overriding options into this instance giving precedence to the overriding options. + * + * @param EncapsulatedOptions $overridingOptions Overriding options. + */ + final public function merge(EncapsulatedOptions $overridingOptions) + { + if (!$overridingOptions instanceof static) { + throw new MergeException('Cannot merge: options must be an instance of "' . get_class($this) . '".'); + } + + $this->options = $this->mergeOptions($overridingOptions, $overridingOptions->options, $this->options); + } + + /** + * Merges the specified overriding options with the options of this instance giving precedence to the overriding + * options. + * + * Overriding this method in a derived class allows it to implement a custom merging strategy using the overriding + * options or the specified explicit overriding options and the specified explicit options of this instance. + * Explicit options are those set explicitly and exclude any default values. + * + * @param EncapsulatedOptions $overridingOptions Overriding options. + * @param array $explicitOverridingOptions Overriding options excluding defaults. + * @param array $explicitOptions This instance's options excluding defaults. + * + * @return array Merged options. + */ + protected function mergeOptions( + EncapsulatedOptions $overridingOptions, + array $explicitOverridingOptions, + array $explicitOptions + ) { + return $overridingOptions->copy() + $this->copy(); + } + /** * Gets the value for the specified option name. Returns the specified * default value if option name is not set. diff --git a/src/Options/MergeException.php b/src/Options/MergeException.php new file mode 100644 index 0000000..e8ba07a --- /dev/null +++ b/src/Options/MergeException.php @@ -0,0 +1,10 @@ + 'bar'], $this->options->copy()); } + public function testMerge() + { + $a = $this->options; + $b = (new TestOptions)->setFoo('bar'); + $c = clone $a; + + self::assertSame('foo', $a->getFoo()); + self::assertSame('bar', $b->getFoo()); + self::assertSame('foo', $c->getFoo()); + + $c->merge($b); + + self::assertSame('foo', $a->getFoo()); + self::assertSame('bar', $b->getFoo()); + self::assertSame('bar', $c->getFoo()); + + $c->merge($a); + + self::assertSame('foo', $a->getFoo()); + self::assertSame('bar', $b->getFoo()); + self::assertSame('foo', $c->getFoo()); + } + + public function testMergeDerivedClass() + { + $this->options->merge(\Mockery::mock(TestOptions::class)); + + // PHPUnit asserts no exception is thrown. + } + + public function testMergeNonDerivedClass() + { + $this->setExpectedException(MergeException::class, TestOptions::class); + + $this->options->merge(\Mockery::mock(EncapsulatedOptions::class)); + } + public function testGetReference() { $this->options->setFoo(['bar' => 'bar', 'baz' => 'baz']); From c36cb50929a9a1c8b41127d66084d18fb40a88ef Mon Sep 17 00:00:00 2001 From: Bilge Date: Sat, 15 Oct 2016 11:40:44 +0100 Subject: [PATCH 2/3] Changed EncapsulatedOptions::mergeOptions to have simplified interface. --- src/Options/EncapsulatedOptions.php | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/Options/EncapsulatedOptions.php b/src/Options/EncapsulatedOptions.php index b4e6ade..d69683e 100644 --- a/src/Options/EncapsulatedOptions.php +++ b/src/Options/EncapsulatedOptions.php @@ -31,28 +31,21 @@ final public function merge(EncapsulatedOptions $overridingOptions) throw new MergeException('Cannot merge: options must be an instance of "' . get_class($this) . '".'); } - $this->options = $this->mergeOptions($overridingOptions, $overridingOptions->options, $this->options); + $this->options = $this->mergeOptions($overridingOptions); } /** * Merges the specified overriding options with the options of this instance giving precedence to the overriding * options. * - * Overriding this method in a derived class allows it to implement a custom merging strategy using the overriding - * options or the specified explicit overriding options and the specified explicit options of this instance. - * Explicit options are those set explicitly and exclude any default values. + * Overriding this method in a derived class allows it to implement a custom merging strategy. * * @param EncapsulatedOptions $overridingOptions Overriding options. - * @param array $explicitOverridingOptions Overriding options excluding defaults. - * @param array $explicitOptions This instance's options excluding defaults. * * @return array Merged options. */ - protected function mergeOptions( - EncapsulatedOptions $overridingOptions, - array $explicitOverridingOptions, - array $explicitOptions - ) { + protected function mergeOptions(EncapsulatedOptions $overridingOptions) + { return $overridingOptions->copy() + $this->copy(); } From 8a8dc36b42bd022c6b8f3d75b4ffedc5231779b4 Mon Sep 17 00:00:00 2001 From: Bilge Date: Sun, 16 Oct 2016 10:44:00 +0100 Subject: [PATCH 3/3] Changed EncapsulatedOptions::merge() to exclude default values. Changed EncapsulatedOptions::set() to reject objects and resources. --- src/Options/EncapsulatedOptions.php | 19 +++++++++------ .../Options/EncapsulatedOptionsTest.php | 24 +++++++++++++++++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/Options/EncapsulatedOptions.php b/src/Options/EncapsulatedOptions.php index d69683e..d91a44c 100644 --- a/src/Options/EncapsulatedOptions.php +++ b/src/Options/EncapsulatedOptions.php @@ -21,7 +21,8 @@ final public function copy() } /** - * Merges the specified overriding options into this instance giving precedence to the overriding options. + * Merges the specified overriding options into this instance giving precedence to the overriding options. Only + * option values that have been explicitly set are merged; that is, defaults are not merged. * * @param EncapsulatedOptions $overridingOptions Overriding options. */ @@ -31,22 +32,22 @@ final public function merge(EncapsulatedOptions $overridingOptions) throw new MergeException('Cannot merge: options must be an instance of "' . get_class($this) . '".'); } - $this->options = $this->mergeOptions($overridingOptions); + $this->options = $this->mergeOptions($this->options, $overridingOptions->options); } /** - * Merges the specified overriding options with the options of this instance giving precedence to the overriding - * options. + * Merges the specified options with the specified overrides giving precedence to the overriding options. * * Overriding this method in a derived class allows it to implement a custom merging strategy. * - * @param EncapsulatedOptions $overridingOptions Overriding options. + * @param array $options Options of this instance. + * @param array $overrides Overriding options. * * @return array Merged options. */ - protected function mergeOptions(EncapsulatedOptions $overridingOptions) + protected function mergeOptions(array $options, array $overrides) { - return $overridingOptions->copy() + $this->copy(); + return $overrides + $options; } /** @@ -90,6 +91,10 @@ final protected function &getReference($option) */ final protected function set($option, $value) { + if (is_object($value) || is_resource($value)) { + throw new \InvalidArgumentException('Value must not be an object or resource.'); + } + $this->options["$option"] = $value; return $this; diff --git a/test/Unit/Porter/Options/EncapsulatedOptionsTest.php b/test/Unit/Porter/Options/EncapsulatedOptionsTest.php index 1a782ae..00a22e0 100644 --- a/test/Unit/Porter/Options/EncapsulatedOptionsTest.php +++ b/test/Unit/Porter/Options/EncapsulatedOptionsTest.php @@ -27,6 +27,20 @@ public function testSet() self::assertSame('bar', $this->options->getFoo()); } + public function testSetObject() + { + $this->setExpectedException(\InvalidArgumentException::class); + + $this->options->setFoo($this->options); + } + + public function testSetResource() + { + $this->setExpectedException(\InvalidArgumentException::class); + + $this->options->setFoo(STDIN); + } + public function testSetNullOverridesDefault() { $this->options->setFoo(null); @@ -53,12 +67,22 @@ public function testMerge() self::assertSame('bar', $b->getFoo()); self::assertSame('foo', $c->getFoo()); + // Merging in b sets c to 'bar'. $c->merge($b); self::assertSame('foo', $a->getFoo()); self::assertSame('bar', $b->getFoo()); self::assertSame('bar', $c->getFoo()); + // Merging in a does not change the value of c because no options have been set explicitly for a. + $c->merge($a); + + self::assertSame('foo', $a->getFoo()); + self::assertSame('bar', $b->getFoo()); + self::assertSame('bar', $c->getFoo()); + + // Merging in a sets c to 'foo' after it has been explicitly set for a. + $a->setFoo('foo'); $c->merge($a); self::assertSame('foo', $a->getFoo());