Przestań wstrzykiwać Entity Managera gdzie popadnie!

znak ze strzałką

Korzystając z paradygmatu obiektowego w tworzeniu programów komputerowych opartych o rozwiązania bazodanowe trzeba zadbać o przełożenie obiektowej struktury na bazę danych. Taki proces nazywany jest ORM (Object Relational Mapping). Na szczęście skończyły się już czasy, kiedy spora część programistów pisała swoje własne mappery.

W świecie otwartego oprogramowania obojętnie jaka technologia posiada kilka popularnych rozwiązań, które dla większości aplikacji powinny wystarczyć. O powyżej wymienionym procesie i konkretnych narzędziach w Sieci znajduje się masa wpisów. Ja natomiast chciałem skupić się na nieco węższym zagadnieniu. Niemniej jednak, czułem że istnieje potrzeba krótkiego wprowadzenia, chociażby dla osób które dopiero zaczynają swoją przygodę z OOP (Object-Oriented Programming).

Przejdźmy do rzeczy

Na co dzień obracam się w ekosystemie PHP, gdzie naturalnym wyborem wydaje się Doctrine ORM – zdecydowanie najpopularniejszy. W tym wpisie wszystkie przykłady odnoszą się właśnie do tej biblioteki, ale jestem pewien, że znajdują one odzwierciedlenie również w innych technologiach. W końcu nikt nie wymyśla koła na nowo, a poszczególne rozwiązania przenikają między ekosystemami.

Centralnym punktem dostępu do funkcjonalności Doctrine ORM jest właśnie Entity Manager – niesamowicie potężne narzędzie, ale niestety wielokrotnie nadużywane. Zaczynając swoją podróż z takimi frameworkami jak Symfony, czy Zend już na samym początku można nadziać się na prosty błąd wrzucania menedżera encji do każdej klasy, w której potrzebujemy komunikacji z bazą danych.

W większości poradników z jakimi miałem do czynienia, na potrzeby prostej aplikacji typu CRUD, prezentowano wstrzyknięcie Entity Managera do kontrolera w celu wywołania metod typu findBy(). Naturalnie, część z nich w dalszej części opierała się o dużo lepsze rozwiązania, ale zakładam, że spore grono programistów poprzestało na pierwszym działającym sposobie. Z przykrością stwierdzę, że sam się na to nabrałem.

Najlepiej na przykładzie

Na warsztat wezmę przytoczony wcześniej przykład i w małych krokach postaram się doprowadzić go do porządku. Chyba nie trzeba tłumaczyć. Prosty kontroler z akcją usuwania zadania należącego do konkretnego autora.

class TaskController extends AbstractRestfulController
{
    private $entityManager;

    public function __construct(EntityManager $entityManager)
    {
        $this->entityManager = $entityManager;
    }

    public function delete($id): JsonModel
    {
        ...

        $task = $this->entityManager
            ->getRepository(Task::class)
            ->findBy([
                'id' => $id,
                'author' => $author
            ]);

        if ($task) {
            $this->entityManager->remove($task);
            $this->entityManager->flush();
        }

        ...
    }
}

Już tak mała ilość kodu pokazuje sedno sprawy. A na co dzień tego typu błędy popełniane są w dużych aplikacjach utrzymywanych latami. Zacznę jednak od początku. Aktualnie jednym z popularniejszych wzorców architektonicznych jest MVC (Model-View-Controller) albo jego pochodne bazujące na podobnych założeniach. Dla adeptów tego właśnie rozwiązania z pewnością znajome jest podejście gruby model, cienki kontroler. Pomijając już fakt czytelności kodu i łatwiejszego testowania, do mnie najbardziej przemawia argument braku logiki biznesowej w kontrolerach. Przekłada się to chociażby na brak potrzeby duplikacji wielu fragmentów, a nawet reużywalność konkretnych kawałków kodu. W powyższym kodzie może akurat tego nie widać, ale wystarczy sobie wyobrazić kolejną regułę biznesową.

Hipotetycznie: Użytkownik może usuwać tylko zadania dodane w ciągu ostatniego tygodnia, w przeciwnym razie istnieje wyłącznie możliwość ich archiwizacji. Kontroler puchnie! Zaraz zaraz, zróbmy akcję grupowego usuwania zadań. Duplikacja kodu! I tak dalej i tak dalej…

To co, może usługa?

class TaskController extends AbstractRestfulController
{
    private $taskManager;

    public function __construct(TaskManager $taskManager)
    {
        $this->taskManager = $taskManager;
    }

    public function delete($id): JsonModel
    {
        ...

        $task = $this->taskManager->findOneByIdAndAuthor($id, $author);

        if ($task) {
            $this->taskManager->delete($task);
        }

        ...
    }
}
class TaskManager
{
    private $entityManager;

    public function __construct(EntityManager $entityManager)
    {
        $this->entityManager = $entityManager;
    }

    public function delete(Task $task): void
    {
        $this->entityManager->remove($task);
        $this->entityManager->flush();
    }

    public function findOneByIdAndAuthor(int $id, UserInterface $author): ?Task
    {
        return $this->entityManager
            ->getRepository(Task::class)
            ->findOneBy([
                'id' => $id,
                'author' => $author
            ]);
    }
}

Dla niektórych niewielka zmiana, ale przyszłościowo na pewno duży krok. Zrefaktoryzowany kod z pewnością rozwiązuje pierwszy napotkany problem, czyli logika w kontrolerze i możliwość duplikacji kodu. Co więcej, nie zostaje złamana pierwsza z zasad SOLID (Single Responsibility Principle) na poziomie metod . Nie mamy też Entity Managera w kontrolerze.

Główny problem przeniesiony został teraz do serwisu. Na czym właściwie polega? Wstrzyknięcie menedżera encji tylko po to żeby wyciągnąć repozytorium w jawny sposób łamie Prawo Demeter. Dla mnie akurat ta reguła nie zawsze ma sens i sam czasem świadomie działam wbrew niej, ale w tym przypadku myślę, że ma znaczenie. Poza tym sam twórca Doctrine nie zaleca używania metody getRepository() w ten sposób. Rozwiązanie nasuwa się samo… wstrzyknięcie repozytorium zamiast Entity Managera. Pozornie mniej wygodne, bo przecież możemy potrzebować większej ilości repozytoriów w danej klasie. Swego czasu też miałem z tym problem, potem jednak zacząłem zauważać, że taka sytuacja jasno sygnalizuje słabo zaprojektowaną architekturę i łamanie już przytoczonej zasady pojedynczej odpowiedzialności. Co więcej, sam kilkukrotnie doświadczyłem problemu wyciągania repozytorium zamiast jego wstrzyknięcia, chociażby w momencie edycji nazwy metody dla całego projektu. Edytor nie rozpoznał, że należy ona do instancji tego repozytorium i nie dokonał modyfikacji. Tego kłopotu można się też pozbyć deklarując typ w specjalnym bloku komentarza, ale nie jest to wymagane, a przy iniekcji nie ma możliwości pomyłki.

Istnieje jeszcze jeden mały problem. Menedżera encji wykorzystuje się głównie w dwóch celach:

  • wyciągnięcie repozytorium;
  • dodawanie/usuwanie obiektów z bazy.

Pierwszy jest z głowy, ale zostaje kwestia modyfikowania. Spokojnie, to też da się ograć w repozytorium nawiązując do Repository Pattern. Finalna wersja kodu zamieszczona poniżej. Puryści czystego kodu pewnie zauważą kilka miejsc do poprawy. Można byłoby pokusić się na przykład o zastosowanie reguły odwracania zależności (D z akronimu SOLID) i implementację interfejsu dla repozytorium. Widziałem już takie rozwiązanie i wydaje się sensowne. Usprawnienie, które jeszcze przychodzi mi na myśl to usunięcie dziedziczenia po EntityRepository i wstrzyknięcie EntityManagera przez konstruktor (kompozycja zamiast dziedziczenia). Poniższy kod ma jednak być najprostszy w swojej postaci i spełniać swoje założenie.

class TaskController extends AbstractRestfulController
{
    private $taskManager;

    public function __construct(TaskManager $taskManager)
    {
        $this->taskManager = $taskManager;
    }

    public function delete($id): JsonModel
    {
        ...

        $this->taskManager->delete($id, $author);

        ...
    }
}
class TaskManager
{
    private $taskRepository;

    public function __construct(TaskRepository $taskRepository)
    {
        $this->taskRepository = $taskRepository;
    }

    public function delete(int $id, UserInterface $author): void
    {
        $task = $this->taskRepository->findCreatedByAuthor($id, $author);

        if ($task === null) {
            throw new \Exception(sprintf('Task with id %d does not exist', $id));
        }

        $this->taskRepository->delete($task);
    }
}
class TaskRepository extends EntityRepository
{
    public function save(Task $task): void
    {
        $this->_em->persist($task);
        $this->_em->flush();
    }

    public function delete(Task $task): void
    {
        $this->_em->remove($task);
        $this->_em->flush();
    }

    public function findCreatedByAuthor(int $id, UserInterface $author): ?Task
    {
        return $this->createQueryBuilder('t')
            ->where('t.id = :id')
            ->andWhere('t.author = :author')
            ->setParameter('id', $id)
            ->setParameter('author', $author)
            ->getQuery()
            ->getOneOrNullResult();
    }
}

Jak zatem prawidłowo korzystać z Doctrine?

Tym krótkim przykładem chciałem pokazać jak ustrzec się podstawowych problemów związanych z wykorzystaniem doctrinowego Entity Managera. W tym miejscu dodam też, że zachowań save() i delete() nie polecam używać w pętli. Wtedy każda iteracja wiązałaby się z zapisem. Zamiast tego można stworzyć metodę saveMultiple() i przekazać wszystkie encje do zapisu w jednej transakcji. Poza bezpośrednim wstrzykiwaniem repozytoriów zachęcam także do pisania własnych metod do wyciągania danych. Dobrze jest działać na kolekcjach, które zawężone zostały już na etapie odpytania bazy danych, tym samym wprowadzając pewną optymalizację.

Sam na początku nie dostrzegałem siły własnych repozytoriów. Teraz nie wyobrażam sobie programowania opierając się tylko na podstawowych metodach doctrinowego interfejsu. Zresztą jak można zauważyć, w przykładzie nie trzeba było wykorzystywać mechanizmu Query Buildera, a zamiast tego można było użyć metody findOneBy(). Tego nie rozpatrywałbym jako błąd, jednak ja osobiście dostrzegam więcej zalet w stworzeniu własnego zapytania. Doctrinowe rozwiązanie wykorzystuje magiczną metodę call() i ma pod spodem sporo magii. W końcu to metoda magiczna. Poza tym własną metodę możemy obdarzyć bardziej wymowną nazwą i ograniczyć w przyszłości zmianę do jednego miejsca, a nie wszystkich w których ją wywołano jak w przypadku tych odziedziczonych po doctrinowym repozytorium.

Spotkałem się także z kontrargumentem, że doctrinowe metody są odpowiednio zabezpieczone przed atakiem typu SQL Injection. To oczywiście nie powoduje żadnej przewagi, gdyż tego rodzaju bezpieczeństwo gwarantują także bindowane w setParameter() wartości.

Jak to jednak w życiu bywa, nie istnieje rozwiązanie posiadające wyłącznie zalety. W praktyce pisanie własnych repozytoriów powoduje, że dosyć szybko puchną. Szczególnie, gdy wielu programistów pracuje w projekcie, a tworzenie reużywalnych kawałków kodu nie idzie zbyt dobrze. W takim wypadku ciężko ustrzec się duplikacji kodu. Trzeba jednak wziąć poprawkę, że to nie problem samego podejścia, ale słabsze wykonanie.

Znajdź optimum

Podsumowując, mam nadzieję, że moje argumenty chociaż trochę dały Wam do myślenia. Czy przedstawione rozwiązanie jest najlepsze? Bardzo możliwe, że nie, ale na dzień dzisiejszy najrozsądniejsze z tych które poznałem. Jeżeli znacie lepszy sposób na korzystanie z Entity Managera, który przestrzega dobrych praktyk programowania obiektowego to bardzo chętnie go poznam.

Zostaje jeszcze kwestia zdrowego rozsądku. Czy nawet jeżeli mam rację to trzeba teraz zmieniać kod w każdym miejscu? Nie wydaje mi się. Sam pracując na swoim starym albo czyimś kodzie często dostosowuję się do tego jak to jest robione w projekcie i nie refaktoryzuję wszystkich miejsc w systemie, a jedynie te w których i tak dokonuję jakiejś modyfikacji. Widziałem gorsze błędy i pewnie sam też takie popełniam, ale mimo wszystko wydaje mi się, że to podejście nie wymaga większego wysiłku, a może w przyszłości zaowocować.

Na koniec serdecznie polecam prezentację Marco Pivetty – twórca Doctrine – o dobrych praktykach tegoż narzędzia.

Krystian Żądło
Programista PHP i właściciel marki Koddlo. Pasjonat czystego kodu i dobrych praktyk programowania obiektowego. Prywatnie fan angielskiej piłki nożnej, dobrego humoru oraz podcastów.