Thanks for the response and for pointing out a few things which I may have glossed over in the article.

I’d like to separate out a few of the arguments to throw in my two cents and for clarity for others reading this.

You mention save being a bad idea and the linked article suggests replacing that with a call to a unit of work and you’ve mentioned calling doctrine’s flush in the controller. Where would you suggest the call to persist lives?

I think the choice for whether to put flush in the repository is highly application-specific. It could go there, it could be in the controller as you suggest. It might also be found in a command bus decorator or Symfony kernel event listener.

I don’t think there’s a general right or wrong here, but the performance and transactional issues you mention are certainly concerns to consider. One other concern you mention is infrastructure leakage.

I’d argue this hasn’t occurred here since our object model has no knowledge of how it might be persisted, no connection to doctrine, and services in our domain layer can call the repository methods (depending on the interface) without knowing how they will be persisted. The implementation of save could be swapped out with a PUT request to an API. No other changes would be required. In fact, I’d also argue that you had you have extracted the flush from the repository and wanted to swap the implementation, you’d have to make the change in your repository and all of your controllers where that’s happening.

Again, I don’t see a right and wrong here, just two opposing ways to go with various trade-offs. Personally, I’d opt for the approach of encapsulating all the logic behind this interface and break it only if a performance need arises that can’t be solved another way.

—

As for the findById method, I think highly coupling a doctrine-specific implementation of a repository class to doctrine itself is perfectly valid and I’d have no concerns about keeping things DRY here. This methods serves as an adapter between the API we want our domain to expose and the API of the persistence layer currently implementing it. That their implementations happen to look very similar to each other might suggest duplication, but it’s very intentional. It’s about allowing the implementation to change without requiring those changes propagate elsewhere.