Repository pattern about apip-ddd

[mnavarrocarter]

Hey @mtarld I appreciate the credits, and sorry for the article being lost. I'm terrible at keeping my blogs! When I created the new one I didn't migrated all the old entries, and this one got lost in the process.

Thanks to you, however, it has been reposted and polished a bit in the one that is my current blog (until I get bored and change it again! 🤣 )

https://blog.mnavarro.dev/the-repository-pattern-done-right

Nice talk at API Platform Conf by the way! 😃

Also @vonalbert if you have any other questions, happy to assist.

[mnavarrocarter]

Hi @vonalbert yes. More specifically, if your read operation's purpose is to just expose the data over an API endpoint or any other infrastructure interface, then that read operation its not a domain read operation, and thus can be outside the domain.

Something I've grown fond of doing is that I have a APIResourceFinder interface (the interface belongs, IMO, in the Infrastructure layer, since its HTTP related). It looks something like this:

<?php

interface APIResourceFinder
{
    public function all(string $resource, array $params = []): array;
    
    public function one(string $resource, string $field, mixed $value): array;
}

This is implemented by something called DoctrineAPIResourceFinder. And there is an implementation that uses the EntityManager. Having the manager there allow us to instrument a few things:

1. A map of resource names to entity classes
2. A custom #[API\Unexposed()] attribute in the Entity class to mark which properties should be ignored. You can use class metadata from doctrine to get the entity reflection instance and fetch the properties attributes.
3. Configure doctrine not to perform any hydration on query.
4. Build a custom query language on top of the request query parameters to include other resources or specify criteria.
5. Hypermedia links for related things thanks to Doctrine metadata too.

Is no simple thing to instrument something like this, and it is also highly opinionated to my particular use case, so let me know if you need to look at an existing implementation.

But basically, the principle behind this is that you don't need a repository to provide data to a JSON endpoint. You do need a repository for your command handlers, because they are the heart of your application and logic and you want the persistence layer there completely abstracted away in there.

All this lives in the infra layer. It's your HTTP layer (Infra) making use of your persistence layer (Infra).

Only drawback I see is that when a resource changes implementation, you need to update it's repository implementation and decorate this ApiResourceFinder to handle that resource differently.

Of course, this is one way of doing it. I have other projects where I use the repository in the controller anyway and let a serializer do the heavy lifting. It's completely up to you and what you need for your particular use case.

[mtarld]

Hey @vonalbert!

Actually, I followed an excellent blog post by @mnavarrocarter, but it seems like it has been removed (maybe Matias could give us the reason why?).
In the meantime, you can find an archive here.

About the inconsistency, you're totally right, but I think it worth it for the sake of performances.

Many thanks for that review!

[vonalbert]

Thank you for pointing me to the right direction.

So, when you say "it worth it for the sake of performances" you mean that:

• if we use the query builder in DoctrineBookRepository::ofId() in order to adhere to the logic of InMemoryBookRepository, we'll get a performance penalty because using the query builder is much slower compared to the find method of entity manager
• if we keep a copy of the InMemoryBookRepository::$entities array (let's call it InMemoryBookRepository::$unfilteredEntities) that it is not filtered, but only updated via save/remove methods in order to adhere to the EntityManagerInterface logic, we'll get a memory penalty during tests (and a less clean logic in that class)

Is that right?

Given that the InMemory repository is only used for testing purposes (and if I were to choose I'd go for the Doctrine repository way) it's maybe a not-so-big issue leaving the things how are now.
However the thing could change if we're going to migrate data from the database table to a remote service. In that case we could have a SomeCoolLibraryWebserviceBookRepository and I'd really want that the behavior of this one adhere completely to the DoctrineBookRepository.

But I think I'm digressing now :) The issue can be closed since my main question was solved!
Thank you again!

[vonalbert]

@mnavarrocarter first of all, thank you for taking the time to re-post the article.

So if I interpreted correctly your final advice, you're suggesting that the read part of the application is not so important from a domain point of view and as such it could suffice to define a service that simply takes the db record and send to output it as raw data.

To generate this raw data you won't use the repository, since the repository is just a collection of the main aggregate, but you use another service. Does this service belong to the domain layer? If yes I suppose that an interface is required. In this case you define a each read model as a distinct object?

And what about the naming of the service above? Would you call it SomeViewModelProvider? SomeViewModelRepository?

[vonalbert]

Thank you for sharing your insight. That was an interesting point of view.

More specifically, if your read operation's purpose is to just expose the data over an API endpoint or any other infrastructure interface, then that read operation its not a domain read operation, and thus can be outside the domain.

Ok, I think I understand. Basically displaying the data over API is a UI concern, so we put that in the infra. But what about those rules that controls which resources I can view? Aren't those related to the domain? For example, take the classic e-commerce application with the classic order aggregate. When a customer request a list of orders should only view a list of its own orders, while the admin can (and should) view all of them. Isn't this rule a domain rule? Or I'm missing something?

On a second thought it's maybe not that important since the rule is enforced in the infrastructure layer anyway (it's the doctrine implementation that performs the query after all).
I think I need to think about it to the implications, but the overall idea makes sense 🙂

let me know if you need to look at an existing implementation.

That would be very appreciated! If you have a public repository I'd love to read the code 🙂

[domis86]

Hi.

I see 3 problems with Repository design here:

1. Its iterator: https://github.com/mtarld/apip-ddd/blob/main/src/Shared/Domain/Repository/RepositoryInterface.php#L12
   • because of this it is treated as "collection of objects" in some situations: https://github.com/mtarld/apip-ddd/blob/main/src/BookStore/Infrastructure/ApiPlatform/State/Provider/BookCollectionProvider.php#L42-L46 . This causes some implicit behaviors, and also is hard to track usages of the iterator methods (how to find all "foreaches" that iterate over some Repository ?).
2. Cloning ( https://github.com/mtarld/apip-ddd/blob/main/src/Shared/Infrastructure/Doctrine/DoctrineRepository.php#L80 etc ) - this causes juggling of instances of Repository. It is not desired behavior of "shared Symfony service" (which resides in container). From such service you expect that you always have one instance of it.
3. Its hybrid of "repository + query builder + paginator". As described in previous 2 points we can see that this Repository is doing too many things. IMO it should only retrieve, save, update and delete entities. But now it also performs role of query builder (related to cloning and returning instance of self etc) and paginator (has state in form of properties page and itemsPerPage). Those functionalities should be extracted to separate classes.

[mnavarrocarter]

Hi @domis86 , sorry I have not read your previous comment, but let me respond to your three points. Hopefully I can do it in a clear way, explaining the reasoning behind any decision and providing an alternative if possible.

1. Regarding the usage of the IteratorAggregate interface, you could not use it. What I've done in some projects is that I've created an interface called Collection that has a collect method, which returns an Iterator of the actual executed query. Implementation is the same as getIterator: it's just a different method. Then, you can do foreach ($users->collect() as $user) and explicitly track the execution of the query. It's not a biggie IMO. Personally, I like the convenience of just Iterate over the collection as I've never felt the need to search for where the iterations happen.

2. The pattern showcased here doesn't adhere to any particular framework idiosyncrasies, so is not a problem if this doesn't meet the expectations of the Symfony container. But, having said that, I don't think this is true anyway: shared service means that the same reference is shared across get calls, but doesn't necessarily mean that you must have only one instance of a particular class, since that would be a singleton and a container cannot enforce such pattern. In other words, the container guarantees to give you the same reference, but it has no business in telling you how many instances you may or may not have of it. You do get the same reference you have in the container every single time in the constructor (so shared service is not violated). What happens after that does not affect the container, nor other services, and cloning is crucial for that, since is a natural API to make this immutable.

3. SRP is about change, not about roles. The class you mention may seem to have many responsibilities (or seem to do many different things) but it has only one reason to change (which is the main goal of SRP). It might violate other principles like interface segregation, but in software engineering, everything is about tradeoffs. Remember that the purpose of the pattern is to emulate collections. If you have ever used any primary OO language (not PHP, because it didn't start as an OO language) you will see that their collection APIs are full of methods to do a myriad of things. Rust's Vector, for example, has probably close to 100 methods that do many different things. Javascript's Array is about the same. Is that breaking SRP? I don't think so.