Repositories inside or outside Domain Services

[webdevilopers]

Came from:

• https://twitter.com/webdevilopers/status/1306286988161617926

I have a service that extracts the MIN and MAX date of a period. This is the original INSIDE approach #1

Domain Service

<?php

namespace Acme\PersonnelManagement\Domain\Service\EmploymentContract;

use Acme\PersonnelManagement\Domain\Model\EmploymentContract\EmploymentContractId;
use Acme\PersonnelManagement\Domain\Model\EmploymentContract\EmploymentPeriod;
use Acme\PersonnelManagement\Presentation\Model\EmploymentContract\TermRepository;
use Webmozart\Assert\Assert;

final class EmploymentPeriodExtractor
{
    /** @var TermRepository */
    private $termRepository;

    public function __construct(TermRepository $termRepository)
    {
        $this->termRepository = $termRepository;
    }

    /**
     * @param EmploymentContractId[] $contractIds
     * @return EmploymentPeriod
     */
    public function fromContractIds(array $contractIds): EmploymentPeriod
    {
        Assert::allIsInstanceOf($contractIds, EmploymentContractId::class);
        Assert::minCount($contractIds, 1);

        $terms = $this->termRepository->ofContractIds(array_map(function (EmploymentContractId $contractId) {
            return $contractId->toString();
        }, $contractIds));

        $employmentPeriods = [];

        foreach ($terms as $term) {
            $employmentPeriods[] = new EmploymentPeriod(
                $term->startDate(), $term->endDate()
            );
        }

        return EmploymentPeriodMerger::merge($employmentPeriods);
    }
}

Application Service (Command Handler)

<?php

namespace Acme\PersonnelManagement\Application\Service\Person;

use Acme\PersonnelManagement\Domain\Service\EmploymentContract\EmploymentPeriodExtractor;

final class ExtractEmploymentPeriodHandler
{
    /** @var EmploymentPeriodExtractor */
    private $extractor;

    public function __construct(EmploymentPeriodExtractor $extractor)
    {
        $this->extractor = $extractor;
    }

    public function __invoke(ExtractEmploymentPeriod $command): void
    {
        $newPeriod = $this->extractor->fromContractIds($command->contractIds());
        // Save aggregate...
    }
}

The domain layer always holds an interface for the TermRepository:

<?php

namespace Acme\PersonnelManagement\Presentation\Model\EmploymentContract;

use Acme\PersonnelManagement\Domain\Model\EmploymentContract\EmploymentContractId;

interface TermRepository
{
    public function ofPersonId(string $personId): array;

    /**
     * @param string[] $contractIds
     * @return Term[]
     */
    public function ofContractIds(array $contractIds): array;
}

The implementation lives inside the infrastructure layer.
Since the Extractor Service only gets the interface type hinted I think it is valid to see it as a Domain Service.

Unfort. this is quite hard to unit test. It would require mocking or a InMemoryTermRepository with same fake data.
It also looks like it is violating the single-responsibility principle (SRP).

This is my OUTSIDE approach #2:

Domain Service

<?php

namespace Acme\PersonnelManagement\Domain\Service\EmploymentContract;

use Acme\PersonnelManagement\Domain\Model\EmploymentContract\EmploymentContractId;
use Acme\PersonnelManagement\Domain\Model\EmploymentContract\EmploymentPeriod;
use Acme\PersonnelManagement\Presentation\Model\EmploymentContract\TermRepository;
use Webmozart\Assert\Assert;

final class EmploymentPeriodExtractor
{
    /**
     * @param Term[] $terms
     * @return EmploymentPeriod
     */
    public function fromTerms(array $terms): EmploymentPeriod
    {
        Assert::allIsInstanceOf($terms, Term::class);
        Assert::minCount($terms, 1);

        $employmentPeriods = [];

        foreach ($terms as $term) {
            $employmentPeriods[] = new EmploymentPeriod(
                $term->startDate(), $term->endDate()
            );
        }

        return EmploymentPeriodMerger::merge($employmentPeriods);
    }
}

Application Service (Command Handler)

<?php

namespace Acme\PersonnelManagement\Application\Service\Person;

use Acme\PersonnelManagement\Domain\Service\EmploymentContract\EmploymentPeriodExtractor;

final class ExtractEmploymentPeriodHandler
{
    /** @var TermRepository */
    private $termRepository;

    /** @var EmploymentPeriodExtractor */
    private $extractor;

    public function __construct(TermRepository $termRepository, EmploymentPeriodExtractor $extractor)
    {
        $this->termRepository = $termRepository;
        $this->extractor = $extractor;
    }

    public function __invoke(ExtractEmploymentPeriod $command): void
    {
        $terms = $this->termRepository->ofContractIds(array_map(function (EmploymentContractId $contractId) {
            return $contractId->toString();
        }, $command->contractIds()));

        $newPeriod = $this->extractor->fromTerms(terms);
        // Save aggregate...
    }
}

This is much easier to test. Though a developer could easily use this code to manipulate the Extractor result by freely passing any Terms as argument. But I guess the developer should not be "the enemy".

Which approach do you prefer? Any exceptions to this or improvements?

Thank you for your feedback.

[cherifGsoul]

I would use the first implementation, I don't think that it (the first example implementation) breaks the SRP principle, since it uses domain components (repository with domain service), yeah using test doubles for testing is fine, integration testing will cover the caveats of unit tests.

I guess even with the second implementation you have to make test double for the repository.