Giter VIP home page Giter VIP logo

Comments (6)

webdevilopers avatar webdevilopers commented on August 16, 2024

The alternative approaches were using a Domain Service to ensure that the developer could not pass "incorrectly determined" entered contracts.

A+ES

final class ServiceContract extends AggregateRoot
{
    public static function enter(
        ServiceContractId $anId, PersonId $anPersonId, PersonalData $aPersonalData,
        AssignmentPeriod $anAssignmentPeriod, WorkweekDays $aWorkweekDays, WeeklyWorkingHours $aWeeklyWorkingHours,
        JobFunctionId $aJobFunctionId, string $aJobFunctionName,
        AgencyId $anAgencyId, string $anAgencyName,
        WorkplaceId $aWorkplaceId, string $aWorkplaceName,
        OverlapDetectorServiceInterface $overlapDetector
    ): ServiceContract
    {
        Assert::notEmpty($aJobFunctionName);
        Assert::notEmpty($anAgencyName);
        Assert::notEmpty($aWorkplaceName);

        $overlapDetector->ofPersonId($anPersonId, $anId, $anAssignmentPeriod);

        $self = new self();
        $self->recordThat(ServiceContractEntered::withData(
            $anId, $anPersonId, $aPersonalData,
            $anAssignmentPeriod, $aWorkweekDays, $aWeeklyWorkingHours,
            $aJobFunctionId, $aJobFunctionName, $anAgencyId, $anAgencyName,
            $aWorkplaceId, $aWorkplaceName, new DateTimeImmutable()
        ));

        return $self;
    }

Service implementation

final class OverlapDetector implements OverlapDetectorServiceInterface
{
    public function ofPersonId(PersonId $personId, ServiceContractId $currentContractId, AssignmentPeriod $currentPeriod): array
    {
        $enteredContracts = $this->contractTermRepository->ofPersonId($personId()->toString());

        foreach ($enteredContracts as $enteredContract) {
            if ($currentPeriod->overlapsWith($enteredContract->assignmentPeriod())) {
                throw new AssignmentPeriodOverlapsException();
            }
        }
    }
}

Alternatively the service could return a bool and the aggregate could throw the exception.
For testing different implementations could be used e.g. "HasConflictDetector" or "HasNoConflictDetector".

Still I find this harder to test and as @plalx mentioned it leaks business logic outside of the aggregate.

A few things to consider: - Logic in infrastructure implies integration tests verification. - Checking in service leaks BL outside the model
The additional argument (data/service) communicates there's a rule to check at least and you can't just set the period to whatever you want. The service may be safer, but compromises the model's purity. If you go with an infrastructure-based impl. I prefer providing the service.
That looks good: hard to misuse the AR method and easy to test the collab between the AR and the detector. The AR is unpure and the detector may not be unit testable, but you can't win everywhere.

from php-ddd.

webdevilopers avatar webdevilopers commented on August 16, 2024

Yet another solution would be to go full event-driven.

As @BrunoRommens suggests:

What are design implications with this ? Maybe one new aggregate "Available periods" providing a "Reserve period" command execution, raising "Period reserved" or "Period not reserved" events. The "Contract" aggregate factory would tell command exec and create or not the contract.

Mabye a Contract should not be entered through itself but through Person. The Person could hold the enteredContracts value objects. The overlapping could be checked. In addition all the personalData mentioned above can come from there too.

final class Person extends AggregateRoot
{
    private $personId;

    private $personalData;

    private $enteredContracts; // From applying "ServiceContractEntered" event from the `ServiceContract` A+ES

    public function enterServiceContract(ServiceContractId $serviceContractId, AssignmentPeriod $period, ...other arguments...): ServiceContract
    {
        foreach ($this->enteredContracts as $enteredContract) {
            if ($period->overlapsWith($enteredContract->assignmentPeriod())) {
                throw new AssignmentPeriodOverlapsException();
            }
        }

        return ServiceContract::enter(
            $serviceContractId
            $this->personId,
            $this->personalData,
            $period,
            WorkweekDays::fromArray(["monday","tuesday","wednesday","thursday","friday"]),
            WeeklyWorkingHours::fromFloat(40),
            JobFunctionId::fromInteger(4), 'Qualitätsprüfer/in',
            AgencyId::fromString('904c0436-4226-4725-afbe-14526906afdb'), 'Office People',
            WorkplaceId::fromString('1e522e02-b28d-44d2-a3fb-4efbdacec462'), 'Bratislava'
        );
    }
}
final class ServiceContract extends AggregateRoot
{
    public static function enter(
        ServiceContractId $anId, PersonId $anPersonId, PersonalData $aPersonalData,
        AssignmentPeriod $anAssignmentPeriod, WorkweekDays $aWorkweekDays, WeeklyWorkingHours $aWeeklyWorkingHours,
        JobFunctionId $aJobFunctionId, string $aJobFunctionName,
        AgencyId $anAgencyId, string $anAgencyName,
        WorkplaceId $aWorkplaceId, string $aWorkplaceName
    ): ServiceContract
    {
        Assert::notEmpty($aJobFunctionName);
        Assert::notEmpty($anAgencyName);
        Assert::notEmpty($aWorkplaceName);

        $self = new self();
        $self->recordThat(ServiceContractEntered::withData(
            $anId, $anPersonId, $aPersonalData,
            $anAssignmentPeriod, $aWorkweekDays, $aWeeklyWorkingHours,
            $aJobFunctionId, $aJobFunctionName, $anAgencyId, $anAgencyName,
            $aWorkplaceId, $aWorkplaceName, new DateTimeImmutable()
        ));

        return $self;
    }

But does this not move the logic of overlapping far away from the developer? He could easily ignore the Person factory method.
Or is this just a matter of communication?

At least the true invariants are still protected inside the ServiceContract. The spanning aggregate rules are protected by the factory on another aggregate (Person).

from php-ddd.

webdevilopers avatar webdevilopers commented on August 16, 2024

Thanky you very much to @BrunoRommens who draw these diagrams to understand the suggested solutions:

1
2a
2b
3a
3b

from php-ddd.

webdevilopers avatar webdevilopers commented on August 16, 2024

4a
4b

This 3rd solution is not exactly what I meant initialy. I think the "non overlaping contract assignment periods" rule deserves its own aggregate, because it only applies to "Service contract assignment periods" and not on other "Person" data. Something like period reservation.
I think it enforces the transactionality of the rule. Checking and choosing a non overlaping assignment period has to be done sequentially. But maybe it is overdesign in your user context (ex: no risk of concurrent calls).
Moreover it will need synchronization. Something like a policy handling the "Assignement period reserved for Service contract" and triggering the "Enter (Service contract)" on the "Service contract Factory".
Ps : more precisely on my previous writing : Checking and choosing a non overlaping assignment period has to be done "atomistically and non concurrently" (in place of "sequentially")

@BrunoRommers

from php-ddd.

webdevilopers avatar webdevilopers commented on August 16, 2024

I like the idea of making the AssignmentPeriodAvailability an AR. Didin't consider it at first b/c I thought there could be thousands or more taken periods, but that's less likely given the segregation by Person.
If scalability isin't an issue then perhaps both ARs could be modified in the same tx to avoid eventual consistency. Domain events could still be leveraged to keep things decoupled (in-memory bus). Eventual consistency can be implemented later if needed without mod. AR bounds.

@plalx

from php-ddd.

webdevilopers avatar webdevilopers commented on August 16, 2024

Here is a more complex example for merging contracts.

final class ServiceContract extends AggregateRoot
{
    public function mergeWith(array $mergeWithContractIds, array $enteredContracts): void
    {
        Assert::minCount($mergeWithContractIds, 2);

        /** @var Details $contractsToMerge */
        $contractsToMerge = [];
        $surroundingAssignmentPeriod = $this->assignmentPeriod;

        foreach ($mergeWithContractIds as $mergeWithContractId) {
            if (!key_exists($mergeWithContractId->toString(), $enteredContracts)) {
                throw new InvalidServiceContractProvidedForMergingException();
            }

            if ($this->isMergedWith($mergeWithContractId)) {
                throw new AlreadyMergedWithServiceContractException();
            }

            $contractToMerge = $enteredContracts[$mergeWithContractId->toString()];

            // Calculate surrounding period for all contracts.
            $surroundingAssignmentPeriod = $surroundingAssignmentPeriod->mergeWith($contractToMerge->assignmentPeriod());

            if ($this->id->sameValueAs(ServiceContractId::fromString($contractToMerge->contractId()))) {
                continue;
            }

            $contractsToMerge[] = $contractToMerge;
        }

        if ($this->assignmentPeriod->startDate()->format('Y-m-d') !== $surroundingAssignmentPeriod->startDate()->format('Y-m-d')) {
            // This is not the initial contract
            throw new InvalidInitialServiceContractForMergingException();
        }

        $overlappingEnteredContractIds = [];

        foreach ($enteredContracts as $enteredContract) {
            if ($enteredContract->assignmentPeriod()->overlapsWith($surroundingAssignmentPeriod)) {
                // Handle overlapping contracts only.
                if (!in_array(ServiceContractId::fromString($enteredContract->contractId()), $mergeWithContractIds)) {
                    // Register overlapping contracts that have not been marked for merging.
                    $overlappingEnteredContractIds[] = $enteredContract->contractId();
                }
            }
        }

        if (0 !== count($overlappingEnteredContractIds)) {
            throw new ServiceContractMergeConflictDetectedException();
        }

        foreach ($contractsToMerge as $contractToMerge) {
            $this->recordThat(MergedWithServiceContract::with(
                $this->id,
                ServiceContractId::fromString($contractToMerge->contractId()),
                $surroundingAssignmentPeriod,
                WorkweekDays::fromArray($contractToMerge->workweekDays()),
                WeeklyWorkingHours::fromFloat($contractToMerge->weeklyWorkingHours()),
                JobFunctionId::fromInteger($contractToMerge->jobFunctionId()),
                $contractToMerge->jobFunctionName(),
                AgencyId::fromString($contractToMerge->agencyId()),
                $contractToMerge->agencyName(),
                WorkplaceId::fromString($contractToMerge->workplaceId()),
                $contractToMerge->workplaceName(),
                new DateTimeImmutable()
            ));
        }
    }
}

While merging per se is a complex process this feels a little bit overloaded for the aggregate root.

That is why I suggested to use the Person aggregate for the overlap detection and then enter the valid contract from there.

There is a similar approach in "Implement #DDDesign" by Vaughn Vernon:

Screenshot from 2020-10-26 15-49-07

The "Forum -> Discussion" example analogous to my "Person- > Contract" example:

Screenshot from 2020-10-26 15-49-48

The question here is: Is Discussion an event-sourced aggregate-root? Since Forum pops the event it does not seem be.

from php-ddd.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.