Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adicionando Event Sourced Aggregates #31

Open
wants to merge 83 commits into
base: master
Choose a base branch
from

Conversation

lucianoqueiroz
Copy link

Substitui o Symfony pelo zend-expressive. Adicionei alguns packages como PhpSpec, Ramsei/Uuid, Beberlei/Assertion e os packages do Prooph. Também comecei a modelar o Event Aggregate, não sei se a estratégia de diretórios que tomei é a melhor, fiz isso mais pra dar um start na pull request.


function it_should_return_datetimes_objects_on_start_and_end_methods()
{
$this->beConstructedThrough('fromStartAndEnd', ['2016-01-01', '2016-04-01']);
Copy link
Member

Choose a reason for hiding this comment

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

same as above, DateTimeImmutable objects please.

Copy link
Author

Choose a reason for hiding this comment

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

Done! 👍

function it_should_return_datetimes_objects_on_start_and_end_methods()
{
$this->beConstructedThrough('fromStartAndEnd', ['2016-01-01', '2016-04-01']);
$this->start()->shouldReturnAnInstanceOf(\DateTime::class);
Copy link
Member

Choose a reason for hiding this comment

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

nop.

Copy link
Author

Choose a reason for hiding this comment

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

Changed to DateTimeImmutable!

{
$this->beConstructedThrough('fromStartAndEnd', ['2016-01-01', '2016-04-01']);
$this->start()->shouldReturnAnInstanceOf(\DateTime::class);
$this->end()->shouldReturnAnInstanceOf(\DateTime::class);
Copy link
Member

Choose a reason for hiding this comment

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

nop. Remember, less state changes you have, better you can trust the system. make it DateTimeImmutable

Copy link
Author

Choose a reason for hiding this comment

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

Changed to DateTimeImmutable! 👍

use Conticket\Model\Events\Event\EventWasCreated;
use Conticket\Model\Events\Event\TicketWasAdded;
use Assert\Assertion;

class Event extends AggregateRoot
Copy link
Member

Choose a reason for hiding this comment

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

final

Copy link
Author

Choose a reason for hiding this comment

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

Done! 👍

use Assert\Assertion;

class Event extends AggregateRoot
{
private $aggregateId;
private $name;
private $description;
private $tickets = [];
Copy link
Member

Choose a reason for hiding this comment

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

Do not use an array, use a collection.

Copy link
Author

Choose a reason for hiding this comment

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

Done! 👍

Assertion::notEmpty($end, "End date is required.");

$start = new \DateTime($start);
$end = new \DateTime($end);
Copy link
Member

Choose a reason for hiding this comment

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

nopeee

return new self($start, $end);
}

public function start()
Copy link
Member

Choose a reason for hiding this comment

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

return type

Copy link
Author

Choose a reason for hiding this comment

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

Done! 👍

return $this->start;
}

public function end()
Copy link
Member

Choose a reason for hiding this comment

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

return type

Copy link
Author

Choose a reason for hiding this comment

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

Done 👍

return $this->end;
}

public function expiresOn(){}
Copy link
Member

Choose a reason for hiding this comment

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

return type

Copy link
Member

Choose a reason for hiding this comment

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

imcomplete


public static function toString($statusCode)
{
return isset(self::$strings[$statusCode]) ? self::$strings[$statusCode] : null;
Copy link
Member

Choose a reason for hiding this comment

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

exception

class EventSpec extends ObjectBehavior
{

function let()
Copy link
Member

Choose a reason for hiding this comment

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

visibility please, do not follow bad practices

Copy link
Author

Choose a reason for hiding this comment

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

Those are auto generated objects by PhpSpec 👊

composer.json Outdated
"friendsofsymfony/rest-bundle": "^1.7",
"jms/serializer-bundle": "^1.0",
"doctrine/doctrine-fixtures-bundle": "^2.2"
"zendframework/zend-expressive": "^1.0",
Copy link
Member

Choose a reason for hiding this comment

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

--sort-packages

composer.json Outdated
"zendframework/zend-expressive": "^1.0",
"ramsey/uuid": "^2.8",
"beberlei/assert": "^2.6",
"prooph/event-sourcing": "~4.0",
Copy link
Member

Choose a reason for hiding this comment

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

^4.0

use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

class EventSpec extends ObjectBehavior
Copy link
Member

Choose a reason for hiding this comment

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

why not final?

]);
}

function it_is_initializable()
Copy link
Member

Choose a reason for hiding this comment

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

visibility and :void


function it_is_initializable()
{
$this->shouldHaveType('Conticket\Model\Aggregates\Event\Event');
Copy link
Member

Choose a reason for hiding this comment

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

use ::class

$this->shouldHaveType('Conticket\Model\Aggregates\Event\Event');
}

function it_should_return_event_id()
Copy link
Member

Choose a reason for hiding this comment

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

visibility and :void

$this->shouldHaveType('Conticket\Model\Aggregates\Event\TicketLifespan');
}

function it_throws_an_exception_when_start_date_is_greater_than_end_date()
Copy link
Member

Choose a reason for hiding this comment

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

visibility and :void


class Event extends AggregateRoot
{
private $aggregateId;
Copy link
Member

Choose a reason for hiding this comment

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

docummentation

return $this->id() === $userId->id();
}

public function toString()
Copy link
Member

Choose a reason for hiding this comment

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

not needed, you have __toString

return $this->id;
}

public function __toString()
Copy link
Member

Choose a reason for hiding this comment

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

missing return type


public function __construct($id = null)
{
$this->id = null === $id ? self::fromString(Uuid::uuid4()->toString()) : $id;
Copy link
Member

Choose a reason for hiding this comment

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

same as above


namespace Conticket\Model\Aggregates\Event;

class TicketEndDateMustBeGreaterThanStartDateException extends \Exception
Copy link
Member

Choose a reason for hiding this comment

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

Put an local interface


class TicketEndDateMustBeGreaterThanStartDateException extends \Exception
{
public function __construct($message = 'Ticket end date must be greater than start date')
Copy link
Member

Choose a reason for hiding this comment

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

do not do it, use named constructors instead

@@ -43,5 +43,6 @@ public function fromIdAndNameAndDescription(TicketId $id, $name, $description)
$ticket->name = $name;
$this->description = $description;
$this->status = TicketStatus::INACTIVE;
return $ticket;
Copy link
Member

Choose a reason for hiding this comment

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

missing an empty line above it.

@@ -43,5 +43,6 @@ public function fromIdAndNameAndDescription(TicketId $id, $name, $description)
$ticket->name = $name;
Copy link
Member

Choose a reason for hiding this comment

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

can you align this assigns?

@@ -0,0 +1,82 @@
<?php
/*
Copy link
Member

Choose a reason for hiding this comment

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

declare(strict_types=1);

Copy link
Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,29 @@
<?php

Copy link
Member

Choose a reason for hiding this comment

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

declare(strict_types=1);

@@ -0,0 +1,55 @@
<?php
/*
Copy link
Member

Choose a reason for hiding this comment

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

declare(strict_types=1);

Copy link
Author

Choose a reason for hiding this comment

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

done

*
* This software consists of voluntary contributions made by many individuals
* and is licensed under the MIT license.
*/
Copy link
Member

Choose a reason for hiding this comment

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

declare(strict_types=1);

Copy link
Author

Choose a reason for hiding this comment

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

done

$this->id = null === $id ? self::fromString(Uuid::uuid4()->toString()) : $id;
}

public static function fromString($id)
Copy link
Member

Choose a reason for hiding this comment

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

string $id

Copy link
Author

Choose a reason for hiding this comment

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

done

{
public static function fromTicketAndNameAndDescription($ticketId, $name, $description)
{
return self::occur($ticketId->toString(), [
Copy link
Member

Choose a reason for hiding this comment

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

(string)

Copy link
Author

Choose a reason for hiding this comment

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

done

{
return self::occur($ticketId->toString(), [
'name' => $name,
'description' => $description
Copy link
Member

Choose a reason for hiding this comment

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

,

Copy link
Author

Choose a reason for hiding this comment

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

done

]);
}

public function name()
Copy link
Member

Choose a reason for hiding this comment

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

same as above

Copy link
Author

Choose a reason for hiding this comment

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

done

return $this->payload['name'];
}

public function description()
Copy link
Member

Choose a reason for hiding this comment

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

same as above


public function name()
{
return $this->payload['name'];
Copy link
Member

Choose a reason for hiding this comment

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

do not leave it on the payload, move it to a private variable :D

Copy link
Author

Choose a reason for hiding this comment

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

what's the problem with the payload? i like him 🕶

@malukenho
Copy link
Member

I'm wondering if we can drop php 7.0 support and work only with 7.1. wdyt?

/cc @abdala @rafaelqueiroz

@abdala
Copy link
Contributor

abdala commented Dec 1, 2016

The project main idea is to apply new thought and technologies. So...7.1 for sure.

@abdala
Copy link
Contributor

abdala commented Dec 1, 2016

@lucianoqueiroz welcome to @malukenho's review!!!!! hahahaha

@abdala
Copy link
Contributor

abdala commented Dec 1, 2016

@lucianoqueiroz Why didn't you delete old files? We are working with a VCS. ;)

@lucianoqueiroz
Copy link
Author

@abdala this guy is a pain in the ass 😩

@lucianoqueiroz
Copy link
Author

@abdala ok, i deleted entire ApiBundle 😄 i was using it to guide the modelling

use Prophecy\Argument;
use Conticket\Model\Aggregates\Event\TicketEndDateMustBeGreaterThanStartDateException;


Copy link
Member

Choose a reason for hiding this comment

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

too much space in here, remove one line


final class TicketLifespanSpec extends ObjectBehavior
{
public function it_is_initializable()
Copy link
Member

Choose a reason for hiding this comment

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

:void

Copy link
Author

Choose a reason for hiding this comment

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

man, this Spec classes get broken when we set the return to :void

$this->value = $value;
}

public static function fromAmount(string $amount, string $code = self::CURRENCY_DEFAULT)
Copy link
Member

Choose a reason for hiding this comment

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

missing return type

{
public static function fromTicketAndStartDateAndEndDate(
Ticket $ticket, \DateTimeImmutable $start, \DateTimeImmutable $end
) : self
Copy link
Member

Choose a reason for hiding this comment

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

cs

Copy link
Author

Choose a reason for hiding this comment

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

man, "cs" to me is counter strike... what do you mean? 😆

Copy link
Author

Choose a reason for hiding this comment

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

i got what means above hehe

@lucianoqueiroz
Copy link
Author

guys, i have to perform some functional tests yet to assure the classes are working well together. I made the previous commit to show how de ticket modeling is going on. other thing i should ask is if we can merge some properties into other value objects for example (price + quantity + lifespam) = TicketDetail or TicketInfo could be good? i also would like to discuss about (name + description) I'm usually getting this kinds of properties in only one like TicketCover or TicketHeadline, TicketBody. Some concept of something that is used to illustrate/inform the user about the Ticket.

@abdala @malukenho


final class Event extends AggregateRoot
{
private $aggregateId;
Copy link
Member

Choose a reason for hiding this comment

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

missing dockblock, mainly for IDE hint

final class Event extends AggregateRoot
{
private $aggregateId;
private $name;
Copy link
Member

Choose a reason for hiding this comment

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

missing dockblock

{
private $aggregateId;
private $name;
private $description;
Copy link
Member

Choose a reason for hiding this comment

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

missing dockblock

private $aggregateId;
private $name;
private $description;
private $banner;
Copy link
Member

Choose a reason for hiding this comment

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

missing dockblock

return $this->aggregateId;
}

public static function fromNameAndDescription($name, $description) : self
Copy link
Member

Choose a reason for hiding this comment

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

missing type hinting public static function fromNameAndDescription(string $name, string $description) : self


public function ticketId() : string
{
return $this->payload['ticked_id'];
Copy link
Member

Choose a reason for hiding this comment

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

do not use payload directly

public static function fromEventAndTicket(Event $event, Ticket $ticket) : self
{
return self::occur((string) $event->aggregateId(), [
'ticket_id' => (string)$ticket->aggregateId()
Copy link
Member

Choose a reason for hiding this comment

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

space missing (string)$ticket->aggregateId() and remember the comma

final class TicketWasCreated extends AggregateChanged
{
public static function fromTicketIdEventIdAndNameAndDescription(
TicketId $ticketId, EventId $eventId, string $name, string $description
Copy link
Member

Choose a reason for hiding this comment

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

It's not allowed by psrs

$this->type = $type;
$this->key = $key;
return self::occur((string) $ticketId, [
'event_id' => (string) $eventId,
Copy link
Member

Choose a reason for hiding this comment

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

align

{
return $this->type;
return $this->payload['name'];
Copy link
Member

Choose a reason for hiding this comment

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

do not use payload directly

@lucianoqueiroz lucianoqueiroz mentioned this pull request Mar 4, 2017
*/
public function __invoke(Request $request, Response $response, callable $out = null)
{
// @todo work with post parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure about this file? I don't think we need to merge this.

Copy link
Author

@lucianoqueiroz lucianoqueiroz May 4, 2017

Choose a reason for hiding this comment

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

We need to improve our tests with real case scenarios. So this class will be probably fixed when we start to work on our scenarios. The merge isn't ready yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants