From 4bdf3f44ed9293ac96aca2d35e2bd732d27cd365 Mon Sep 17 00:00:00 2001 From: tgallice Date: Wed, 10 Aug 2016 20:57:48 +0200 Subject: [PATCH 1/2] Define value object as final and immutable --- spec/ConversationSpec.php | 9 ++-- spec/EntityApiSpec.php | 12 ++++-- spec/Exception/ConversationExceptionSpec.php | 4 +- spec/Model/ContextSpec.php | 43 ++++++++++++++------ spec/Model/EntitySpec.php | 4 +- src/Model/Context.php | 23 ++++++++--- src/Model/Entity.php | 2 +- src/Model/EntityValue.php | 2 +- src/Model/Location.php | 2 +- 9 files changed, 70 insertions(+), 31 deletions(-) diff --git a/spec/ConversationSpec.php b/spec/ConversationSpec.php index 3330fd2..28c1be4 100644 --- a/spec/ConversationSpec.php +++ b/spec/ConversationSpec.php @@ -49,8 +49,7 @@ function it_converse_automatically_and_return_the_last_context($api, $actionMapp { $api->converse('session_id', 'my text', Argument::any())->willReturn($this->stepData[Step::TYPE_MERGE]); - $expectedContext = new Context(); - $expectedContext->add('custom', 'value'); + $expectedContext = new Context(['custom' => 'value']); $actionMapping ->merge('session_id', Argument::type(Context::class), $this->stepData[Step::TYPE_MERGE]['entities']) @@ -149,8 +148,7 @@ function it_delegate_action_step_execution($api, $actionMapping) { $api->converse('session_id', 'my text', Argument::any())->willReturn($this->stepData[Step::TYPE_ACTION]); - $expectedContext = new Context(); - $expectedContext->add('custom', 'value'); + $expectedContext = new Context(['custom' => 'value']); $actionMapping ->action('session_id', $this->stepData[Step::TYPE_ACTION]['action'], Argument::type(Context::class), Argument::type('array')) @@ -197,8 +195,7 @@ function it_delegate_merge_execution($api, $actionMapping) { $api->converse('session_id', 'my text', Argument::any())->willReturn($this->stepData[Step::TYPE_MERGE]); - $expectedContext = new Context(); - $expectedContext->add('custom', 'value'); + $expectedContext = new Context(['custom' => 'value']); $actionMapping ->merge('session_id', Argument::type(Context::class), $this->stepData[Step::TYPE_MERGE]['entities']) diff --git a/spec/EntityApiSpec.php b/spec/EntityApiSpec.php index 4099f62..5c0657a 100644 --- a/spec/EntityApiSpec.php +++ b/spec/EntityApiSpec.php @@ -41,7 +41,7 @@ function it_should_get_entities($client, ResponseInterface $response) $this->get()->shouldReturn($expected); } - function it_should_create_entities($client, ResponseInterface $response, EntityValue $entityValue) + function it_should_create_entities($client, ResponseInterface $response) { $body = ' { @@ -54,6 +54,8 @@ function it_should_create_entities($client, ResponseInterface $response, EntityV } '; + $entityValue = new EntityValue('value'); + $expected = json_decode($body, true); $response->getBody()->willReturn($body); $client->post('/entities', [ @@ -100,7 +102,7 @@ function it_should_get_an_entity($client, ResponseInterface $response) $this->get('first_name')->shouldReturn($expected); } - function it_should_update_an_entity($client, ResponseInterface $response, EntityValue $entityValue) + function it_should_update_an_entity($client, ResponseInterface $response) { $body = ' { @@ -113,6 +115,8 @@ function it_should_update_an_entity($client, ResponseInterface $response, Entity } '; + $entityValue = new EntityValue('value'); + $expected = json_decode($body, true); $response->getBody()->willReturn($body); $client->put('/entities/favorite_city', [ @@ -139,7 +143,7 @@ function it_should_delete_an_entity($client, ResponseInterface $response) $this->delete('favorite_city')->shouldReturn($expected); } - function it_should_add_a_value_to_an_entity($client, ResponseInterface $response, EntityValue $entityValue) + function it_should_add_a_value_to_an_entity($client, ResponseInterface $response) { $body = ' { @@ -153,6 +157,8 @@ function it_should_add_a_value_to_an_entity($client, ResponseInterface $response } '; + $entityValue = new EntityValue('value'); + $expected = json_decode($body, true); $response->getBody()->willReturn($body); $client->send('POST', '/entities/favorite_city/values', $entityValue)->willReturn($response); diff --git a/spec/Exception/ConversationExceptionSpec.php b/spec/Exception/ConversationExceptionSpec.php index 7ebad22..1f50f1e 100644 --- a/spec/Exception/ConversationExceptionSpec.php +++ b/spec/Exception/ConversationExceptionSpec.php @@ -40,8 +40,10 @@ function it_has_no_context_by_default() $this->getContext()->shouldReturn(null); } - function it_can_have_a_context(Context $context) + function it_can_have_a_context() { + $context = new Context(); + $this->beConstructedWith('error message', null, $context); $this->getContext()->shouldReturn($context); } diff --git a/spec/Model/ContextSpec.php b/spec/Model/ContextSpec.php index 92679e2..2eb46f5 100644 --- a/spec/Model/ContextSpec.php +++ b/spec/Model/ContextSpec.php @@ -4,6 +4,7 @@ use PhpSpec\ObjectBehavior; use Prophecy\Argument; +use Tgallice\Wit\Model\Context; use Tgallice\Wit\Model\Entity; use Tgallice\Wit\Model\Location; @@ -16,7 +17,7 @@ function it_is_initializable() function it_has_a_reference_date() { - $date = new \DateTime(); + $date = new \DateTimeImmutable(); $this->beConstructedWith([ 'reference_date' => $date, @@ -34,8 +35,10 @@ function it_has_no_default_location() $this->getLocation()->shouldReturn([]); } - function it_can_define_a_location(Location $location) + function it_can_define_a_location() { + $location = new Location(1.1, 1.2); + $this->beConstructedWith([ 'location' => $location, ]); @@ -60,8 +63,10 @@ function it_has_no_default_entities() $this->getEntities()->shouldReturn([]); } - function it_can_define_entities(Entity $entity) + function it_can_define_entities() { + $entity = new Entity('id'); + $this->beConstructedWith([ 'entities' => [$entity], ]); @@ -83,23 +88,37 @@ function it_can_define_timezone() function it_can_add_custom_context_field() { - $this->add('context', 'value'); - $this->get('context')->shouldReturn('value'); + $context = $this->add('context', 'value'); + $context->get('context')->shouldReturn('value'); } function it_can_remove_a_context_field() { - $this->add('context', 'value'); - $this->get('context')->shouldReturn('value'); - $this->remove('context'); - $this->get('context')->shouldReturn(null); + $context = $this->add('context', 'value'); + $context->get('context')->shouldReturn('value'); + + $context = $this->remove('context'); + $context->get('context')->shouldReturn(null); } function it_can_check_presence_of_a_context_field() { - $this->add('context', 'value'); - $this->has('context')->shouldReturn(true); - $this->has('wrong_context')->shouldReturn(false); + $context = $this->add('context', 'value'); + $context->has('context')->shouldReturn(true); + $context->has('wrong_context')->shouldReturn(false); + } + + function it_is_immutable() + { + $context = $this->add('context', 'value'); + $newContext = $context->add('context', 'value'); + $newContext2 = $newContext->remove('context'); + + $context->shouldHaveType(Context::class); + $context->shouldBeLike($newContext); + $context->shouldNotBeEqualTo($newContext); + $newContext->get('context')->shouldReturn('value'); + $newContext2->has('context')->shouldReturn(false); } function it_must_be_json_serializable() diff --git a/spec/Model/EntitySpec.php b/spec/Model/EntitySpec.php index 7bebee8..0a02aee 100644 --- a/spec/Model/EntitySpec.php +++ b/spec/Model/EntitySpec.php @@ -29,8 +29,10 @@ function it_has_a_description() $this->getDescription()->shouldReturn('description'); } - function it_has_values(EntityValue $entityValue) + function it_has_values() { + $entityValue = new EntityValue('value'); + $this->beConstructedWith('id', [$entityValue]); $this->getValues()->shouldEqual([$entityValue]); } diff --git a/src/Model/Context.php b/src/Model/Context.php index da763b6..c895844 100644 --- a/src/Model/Context.php +++ b/src/Model/Context.php @@ -2,7 +2,10 @@ namespace Tgallice\Wit\Model; -class Context implements \JsonSerializable +/** + * Context Value Object + */ +final class Context implements \JsonSerializable { /** * @var array @@ -16,10 +19,10 @@ public function __construct($data = []) { // Ensure the refenre_date field if (empty($data['reference_date'])) { - $data['reference_date'] = new \DateTime(); + $data['reference_date'] = new \DateTimeImmutable(); } - if ($data['reference_date'] instanceof \DateTime) { + if ($data['reference_date'] instanceof \DateTimeInterface) { $data['reference_date'] = $data['reference_date']->format(DATE_ISO8601); } @@ -71,10 +74,15 @@ public function getTimezone() /** * @param string $name * @param mixed $value + * + * @return Context */ public function add($name, $value) { - $this->data[$name] = $value; + $newData = $this->data; + $newData[$name] = $value; + + return new self($newData); } /** @@ -89,10 +97,15 @@ public function get($name) /** * @param string $name + * + * @return Context */ public function remove($name) { - unset($this->data[$name]); + $newData = $this->data; + unset($newData[$name]); + + return new self($newData); } /** diff --git a/src/Model/Entity.php b/src/Model/Entity.php index e630faa..27b8253 100644 --- a/src/Model/Entity.php +++ b/src/Model/Entity.php @@ -2,7 +2,7 @@ namespace Tgallice\Wit\Model; -class Entity implements \JsonSerializable +final class Entity implements \JsonSerializable { const LOOKUP_TRAIT = 'trait'; diff --git a/src/Model/EntityValue.php b/src/Model/EntityValue.php index 04e815b..39c1834 100644 --- a/src/Model/EntityValue.php +++ b/src/Model/EntityValue.php @@ -2,7 +2,7 @@ namespace Tgallice\Wit\Model; -class EntityValue implements \JsonSerializable +final class EntityValue implements \JsonSerializable { /** * @var string diff --git a/src/Model/Location.php b/src/Model/Location.php index 9b6fa03..f1dd85c 100644 --- a/src/Model/Location.php +++ b/src/Model/Location.php @@ -2,7 +2,7 @@ namespace Tgallice\Wit\Model; -class Location implements \JsonSerializable +final class Location implements \JsonSerializable { /** * @var float From 7ac624d90e454a3d000751e0a33f1862622e0976 Mon Sep 17 00:00:00 2001 From: tgallice Date: Wed, 10 Aug 2016 21:48:23 +0200 Subject: [PATCH 2/2] Rename Context:add() to Context::set() --- spec/Model/ContextSpec.php | 37 +++++++++++++++++++------------------ src/Model/Context.php | 2 +- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/spec/Model/ContextSpec.php b/spec/Model/ContextSpec.php index 2eb46f5..f382755 100644 --- a/spec/Model/ContextSpec.php +++ b/spec/Model/ContextSpec.php @@ -10,6 +10,11 @@ class ContextSpec extends ObjectBehavior { + function let() + { + $this->beConstructedWith(['field' => 'value']); + } + function it_is_initializable() { $this->shouldHaveType('Tgallice\Wit\Model\Context'); @@ -86,39 +91,35 @@ function it_can_define_timezone() $this->getTimezone()->shouldReturn('timezone'); } - function it_can_add_custom_context_field() + function it_can_set_custom_context_field() { - $context = $this->add('context', 'value'); - $context->get('context')->shouldReturn('value'); + $context = $this->set('custom', 'value'); + $context->get('custom')->shouldReturn('value'); } function it_can_remove_a_context_field() { - $context = $this->add('context', 'value'); - $context->get('context')->shouldReturn('value'); + $this->get('field')->shouldReturn('value'); - $context = $this->remove('context'); - $context->get('context')->shouldReturn(null); + $context = $this->remove('field'); + $context->get('field')->shouldReturn(null); } function it_can_check_presence_of_a_context_field() { - $context = $this->add('context', 'value'); - $context->has('context')->shouldReturn(true); - $context->has('wrong_context')->shouldReturn(false); + $this->has('field')->shouldReturn(true); + $this->has('wrong_context')->shouldReturn(false); } function it_is_immutable() { - $context = $this->add('context', 'value'); - $newContext = $context->add('context', 'value'); - $newContext2 = $newContext->remove('context'); + $newContext = $this->set('field', 'value'); + $newContext2 = $newContext->remove('field'); - $context->shouldHaveType(Context::class); - $context->shouldBeLike($newContext); - $context->shouldNotBeEqualTo($newContext); - $newContext->get('context')->shouldReturn('value'); - $newContext2->has('context')->shouldReturn(false); + $this->shouldBeLike($newContext); + $this->shouldNotBeEqualTo($newContext); + $newContext->get('field')->shouldReturn('value'); + $newContext2->has('field')->shouldReturn(false); } function it_must_be_json_serializable() diff --git a/src/Model/Context.php b/src/Model/Context.php index c895844..fcf0a3e 100644 --- a/src/Model/Context.php +++ b/src/Model/Context.php @@ -77,7 +77,7 @@ public function getTimezone() * * @return Context */ - public function add($name, $value) + public function set($name, $value) { $newData = $this->data; $newData[$name] = $value;