DevCezz

Programistyczny blog dla Ciebie

@Autowired na polu + testy jednostkowe = PORAŻKA
Testy

@Autowired na polu + testy jednostkowe = PORAŻKA

EDIT: Zachęcam do przeczytania sekcji komentarzy, gdzie michaldo podzielił się swoim, jakże ciekawym i owocnym, punktem widzenia w sprawie tego artykułu.

Ostatnio w pracy spędziłem sporo czasu nad jednym zadaniem przez… test jednostkowy. Pomimo tego, że lubię pisać testy to nie dziwię się innym programistom, że w takiej sytuacji zaprzestają ich pisania. Jednak to nie jest wina koncepcji testów tylko tworzenia kodu produkcyjnego, który po prostu nie chce dać się przetestować. Zarysuję Ci teraz jak wyglądała mniej więcej sytuacja.

Zdefiniowanie problemu

Zacznijmy od bazowej klasy abstrakcyjnej jaką zastałem. Oczywiście kod jest zupełnie inny niż w mojej pracy jeśli chodzi o przypadek biznesowy. Ja tylko chcę zarysować problem, który napotkałem.

import org.springframework.beans.factory.annotation.Autowired;

public abstract class UserVerification {

    @Autowired
    private UserService userService;

    public void verifyUser(User user) {
        userService.checkDept(user);
    }
}

Od razu powinno zapalić się światełko w głowie… Już nie wiem od jak dawna nie powinno robić się wstrzykiwania zależności przez pole. Jednak załóżmy, że tego nie zauważyłem i brnąłem w to dalej. Utworzyłem, więc swoją klasę rozszerzającą to bazowe rozwiązanie.

import org.springframework.stereotype.Component;

@Component
public class VipUserVerification extends UserVerification {

    private final VipService vipService;

    public VipUserVerification(final VipService vipService) {
        this.vipService = vipService;
    }

    public void verifyVip(Vip vip) {
        verifyUser(vip);
        vipService.checkExtraDept(vip);
    }
}

Rozszerzyłem UserVerification, dodałem konstruktor wstrzykujący jako parametr VipService i przypisujący je do odpowiedniego pola. Następnie w metodzie verifyVip wywołałem metodę verifyUser z naszej klasy bazowej i metodę checkExtraDept z serwisu. Wszystko jak do tej pory powinno działać jak należy. Oczywiście w tym przypadku nie jest ważne czym jest UserService, VipService, User czy Vip. No dobra to zabierzmy się teraz za clue, czyli nasz test jednostkowy.

Test jednostkowy

Załóżmy, że UserService i VipService to usługi zewnętrzne, więc wypada je zamockować. Zaraz do tego dojdziemy, najpierw zdefiniujmy test.

...
import org.junit.jupiter.api.Test;

import static org.mockito.Mockito.verify;

class VipVerificationTest {

    // ... set up

    @Test
    public void ourTestMethod() {
        Vip vip = new Vip();

        vipUserVerification.verifyVip(vip);

        verify(userService).checkDept(vip);
        verify(vipService).checkExtraDept(vip);
    }
}

Tworzymy nowego Vip’a, wywołujemy weryfikację i sprawdzamy czy nasze usługi zewnętrzne zostały wywołane z naszym obiektem. Prosty test. Wróćmy jednak do ustawień. Musimy utworzyć instancję naszej testowanej klasy i wstrzyknąć jej odpowiedniego mocka.

Pierwsze podejście

Spróbujmy na początek coś takiego.

...
import org.junit.jupiter.api.BeforeEach;

import static org.mockito.Mockito.mock;

class VipVerificationTest {

    private final UserService userService = mock(UserService.class);
    private final VipService vipService = mock(VipService.class);

    private VipUserVerification vipUserVerification;

    @BeforeEach
    public void setUp() {
        vipUserVerification = new VipUserVerification(
                vipService
        );
    }

    // ... test
}

Pomimo tego, że VipUserVerification wymaga tylko VipService to i tak utworzyliśmy mocka UserService, aby sprawdzić czy jego metoda też została wywołana w teście. Dostajemy, więc kolejny alert, ale znowu go umyślnie nie zauważamy. Uruchamiamy test, iiii…. cyk!

java.lang.NullPointerException: Cannot invoke "... .UserService.checkDept(pl.devcezz.javadockerwar.User)" because "this.userService" is null

Dostajemy NullPointerException! Nasz UserService okazał się nullem pomimo tego, że go utworzyliśmy. No dobrze, pewnie to nasz błąd przy ustawieniach testu, spróbujmy czegoś innego.

Podejście drugie

Użyjmy adnotacji Mockito.

...
import org.junit.jupiter.api.BeforeEach;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;

class VipVerificationTest {

    @Mock
    private UserService userService;

    @Mock
    private VipService vipService;

    @InjectMocks
    private VipUserVerification vipUserVerification;

    @BeforeEach
    public void setUp() {
        MockitoAnnotations.openMocks(this);
    }

    // ... test
}

Podejście numer dwa, uruchamiamy test iii… znowu to samo!

java.lang.NullPointerException: Cannot invoke "... .UserService.checkDept(pl.devcezz.javadockerwar.User)" because "this.userService" is null

I teraz irytacja rośnie do granic możliwości. Usuwamy test, robimy commit, push i robota skończona! Tak pewnie zrobiłaby większość z nas. Jednak może warto się chwilę wstrzymać i przenalizować co może być nie tak. Dałem już na początku wskazówkę, więc podążmy ją i spójrzmy na kod produkcyjny.

Poprawka w kodzie produkcyjnym

Powiedziałem, że nie powinno się wstrzykiwać zależności przez pole. Zmieńmy zatem klasę UserVerification, aby odbywało się wstrzykiwanie przez konstruktor.

public abstract class UserVerification {

    private final UserService userService;

    UserVerification(final UserService userService) {
        this.userService = userService;
    }

    public void verifyUser(User user) {
        userService.checkDept(user);
    }
}

Przez tą zmianę wymagane jest poprawienia naszej klasy biznesowej.

import org.springframework.stereotype.Component;

@Component
public class VipUserVerification extends UserVerification {

    private final VipService vipService;

    public VipUserVerification(
            final UserService userService, 
            final VipService vipService
    ) {
        super(userService);
        this.vipService = vipService;
    }

    public void verifyVip(Vip vip) {
        verifyUser(vip);
        vipService.checkExtraDept(vip);
    }
}

Musieliśmy i w konstruktorze klasy VipUserVerification dodać zależność do UserService. Teraz uruchamiamy test i jak ręką odjął! Mamy zielony pasek!

Zakończenie mojej historii

W przypadku mojej pracy nie było happy end’u. Istniało bodajże osiem klas, które rozszerzały tą klasę bazową. Podjąłem się wyzwania i zmieniłem klasę bazową, aby zależności były wstrzykiwane przez konstruktor. To samo musiałem zrobić w klasach ją rozszerzających, ponieważ również zawierały ten błąd. Poprawiłem testy i przechodziły jak należy. Jednak podczas uruchamiania kontekstu aplikacji dostawałem wyjątek o zależności cyklicznej Spring’a. Szukałem błędu, ale deadline’y goniły i musiałem zaprzestać refaktoryzacji. Wróciłem, więc do rozwiązania, które znalazłem w innych klasach. Musiałem zastosować w swojej klasie… wstrzykiwanie przez pole.

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

@Component
public class VipUserVerification extends UserVerification {

    @Autowired
    private VipService vipService;

    public void verifyVip(Vip vip) {
        verifyUser(vip);
        vipService.checkExtraDept(vip);
    }
}

Wtedy w test w takiej postaci przechodzi na zielono. Co prawda zostaje pewien niedosyt i niesmak.

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;

import static org.mockito.Mockito.verify;

class VipVerificationTest {

    @Mock
    private UserService userService;

    @Mock
    private VipService vipService;

    @InjectMocks
    private VipUserVerification vipUserVerification;

    @BeforeEach
    public void setUp() {
        MockitoAnnotations.openMocks(this);
    }

    @Test
    public void ourTestMethod() {
        Vip vip = new Vip();

        vipUserVerification.verifyVip(vip);

        verify(userService).checkDept(vip);
        verify(vipService).checkExtraDept(vip);
    }
}

Podsumowanie

W tym przypadku finał jest taki, że musiałem zastosować coś czego próbuję unikać jak ognia. Musiałem zrezygnować z własnych przekonań, aby dokończyć swoje zadanie. To rozwiązanie wydaje się nieintuicyjne i wręcz nie jest jasne. Wszystko dzieje się za naszymi plecami dzięki magii. Nie mamy nad niczym kontroli. Nie wiemy co dokładnie się dzieje, co od czego zależy. Nawet IDE w postaci IntelliJ jest z nami w tej walce mówiąc nam, że „Field injection is not recommended”. Dlatego ważne jest, aby chwilę się zastanowić nad pisanym przez siebie kodem i sprawdzić czy czasem komuś nie utrudnimy nim pracy.

Jakie jest Twoje zdanie na ten temat? Z chęcią dowiem się czy miewałeś/aś takie same przypadki i jak sobie z nimi radziłeś/aś.

Podziel się tym z innymi!

4 KOMENTARZE

  1. „Już nie wiem od jak dawna nie powinno robić się wstrzykiwania zależności przez pole”

    Zastanówmy się w jakim zakresie to przekonanie jest prawdziwe. Wszak wstrzykiwanie przez konstrukor ma wady:
    – boilerplate
    – zmiana konstuktora klasy bazowej wymaga zmiany we wszystkich klasach pochodnych
    – wstrzykiwanie konstruktorem nakłada dodatkowe wektory zależności i może doprowadzić do cykli, jak w podanym przykładzie

    Wstrzykiwanie konstruktorem natomiast daje nam możliwość utworzenia instancji bez trików w testach automatycznych. Zastanówmy sie więc jakiego typu obiekty zarządzane przez Springa chcielibyśmy utworzyć ręcznie.

    Czy klasa @Configuration powinna być inicjowana przez konstruktor? Uważam że nie, taka klasa poza kontekstem Springa nie ma własnego życia

    Czy klasa @RestController powinna być inicjowania przez konstruktor? Również uważam że nie, kontroler jest takim „rzepem” do które wpinają się poprzez adnotacje procesory Springa które transformują obiekty Javy na protokół HTTP. Analogicznie, poza kontekstem Springa kontrolery nie mają wielkiego sensu.

    Pozostają więc @Service. Czy chcielibyśmy mieć serwisy z konstruktorami aby jest tworzyć na potrzeby testów?

    Aby odpowiedzieć na to pytanie załóżmy okulary DDD. W tej optyce kod dzieli się na kod domenowy z logiką biznesową i kod aplikacji z logiką przepływu procesu i współpracy z innymi systemami.
    Kod domenowy testuje się jednostkowo a kod aplikacyjny integracyjnie. Serwisy Springa należą kodu aplikacyjnego, powinny być testowane integracyjnie a więc w ramach kontekstu Springa. Zatem możliwość ich ręcznego tworzenia przez konstruktory nie jest konieczna.

    Podam teraz przykład kiedy wstrzykiwanie zależności konstruktorem ma sens. Założmy że mamy serwis który potrafi deszfrować dane korzystając z obowiązującego klucza. Załóżmy że klucz jest beanem Springa. Mimo to lepiej użyć konstruktora, gdyż akurat taki deszyfrator chciałbym móc testować jednostkowo. W konwencji DDD taki desztyfrator można uznać za serwis domenowy, a takie należą od obszaru testowanego jednostkowo.

    (W wypadku deszyfratora za optymalne uważam osobną klasę deszyfratora bez adnotacji Springa oraz klasę konfiguracyjną która utworzy deszyfrator jako bean)

    Podsumowując, jako przyczynę tytułowej porażki uważam nie sposób korzystania ze Springa w aplikacji ale przyjętą strategię testowania.

    • Dziękuję michaldo za przedstawienie Twojego punktu widzenia w temacie wstrzykiwania zależności.

      Doceniam, że podzieliłeś się ze mną i z innymi tak rozbudowanym wpisem. Zgadzam się z Tobą, że zdanie napisane przeze mnie w artykule, które przytoczyłeś, może jest zbyt zdecydowane i za bardzo uogólnia wszystkie przypadki użycia. Jak zawsze w naszej branży powinno się napisać: „To zależy” 🙂
      Jednak co do paru kwestii się nie zgodzę, ponieważ wstrzykując przez pole uzależniamy się od wybranego frameworka tak bardzo, że ciężko będzie nam go zmienić na innego rozwiązanie. Przy wstrzykiwaniu przez konstruktor dostajemy swego rodzaju niezależność, ponieważ możemy rejestrować beany wykorzystując adnotację `@Bean`. Łatwiej też nam jest panować nad wszystkimi zależnościami w naszym kodzie i nie tworzyć `god class`, ponieważ widzimy w konstruktorze ile zależności jest potrzebnych, aby utworzyć np. wybrany serwis. Przy korzystaniu z `@Autowired` na polu możemy łatwo się zatracić i dorzucać kolejne zależności jak do worka bez dna. Jeżeli też coś będzie wymagało przetestowania jednostkowego to staje się to dla nas o wiele przyjemniejsze, ponieważ łatwiej jest nam tworzyć testowane obiekty. Jest to po prostu bardziej intuicyjne niż zagłębianie się w zagadnienia refleksji czy też wybrany framework testowy.

      Nie mniej jednak Twoje uzasadnienia też do mnie trafiają i na pewno warto, aby ktoś czytając mój artykuł wsparł się Twoim wpisem. Dlatego z tego miejsca chciałbym Ci bardzo podziękować i życzyć wszystkiego co najlepsze!

  2. Dziękują za miłe słowa – widać warsztat profesjonalnego blogera!

    Można spotkać opinię w sieci, że stosowanie @Autowired sprzyja tworzeniu God-class a stosowanie konstruktorów temu zapobiega, gdyż konstuktory wymagają więcej kodu.
    W skrócie
    God-class + constructor-injection => SOLID-class
    Byłby to argument przeciwko field-injection, ale moje doświadczenie jest inne
    God-class + constructor-injection => God-class + God-constructor

    Faktycznie, dzięki @Autowired łatwo się tworzy God klasy, ale konstruktory nikogo – mnie również – od tego nie odwiodą.

    I jeszcze kilka spostrzeżeń:
    a) „wstrzykując przez pole uzależniamy się od wybranego frameworka tak bardzo, że ciężko będzie nam go zmienić na innego rozwiązanie”

    Komendą `sed` każde wystąpienie @Autowired zastąpić przez javax.inject.Inject i otrzymać aplikację gotową do uruchomienia w innym frameworku który honoruje standardową adnotację @Inject.
    (Oczywiście to są rozważania akademickie gdyż w prawdziwym życiu częstotliwość przenoszenia aplikacji pomiędzy fraweworkami jest zbliżona do zera a zakres zmiana jest dużo większy niż zastąpienie @Autowired przez coś innego).

    b) „Jeżeli też coś będzie wymagało przetestowania jednostkowego to staje się to dla nas o wiele przyjemniejsze, ponieważ łatwiej jest nam tworzyć testowane obiekty. Jest to po prostu bardziej intuicyjne niż zagłębianie się w zagadnienia refleksji czy też wybrany framework testowy.”

    Oczywiście, jeżeli są sytuacje gdy chcemy serwisy testować jednostkowo to powinny mieć normalne konstruktory. Natomiast uważam że w typowym scenariuszu serwisy powinny byc testowane integracyjnie w ramach kontekstu Springa, a wyjaśnienia, że wymaga to znajomości Spring TestContext Framework po prostu nie przyjmuję

    • Również dziękuję za miłe słowa!

      Ciężko się z Tobą nie zgodzić. Faktycznie jeżeli komuś nie przeszkadza ilość pól z adnotacją `@Autowired` to czemu miałoby mu też nie przeszkadzać dodanie kolejnego parametru do konstruktora 🤔. Trzeba mieć nadzieję, że może w tym przypadku istnieje mniejsze ryzyko, ale sam w to wątpię.

      Jeśli chodzi o przechodzenie z frameworka na framework to w dojrzałych aplikacjach opartych na Springu nie ma to faktycznie najmniejszego sensu. Jednak zawsze fajnie jest mieć możliwość zmiany niż jej nie mieć. W sumie ciekawy sposób z tym `@Inject`, ale przypuszczam, że nie byłoby to takie proste 😉.

      Co do ostatniego punktu to pewnie już zależy od higieny pracy i przyzwyczajeń. Jeżeli jest tam jakaś logika i chcemy mieć szybki feedback to może faktycznie lepiej mieć możliwość napisania kilku testów jednostkowych.

      Naprawdę wielkie dzięki za interesującą wymianę zdań. Widać, że jeszcze z niewielu pieców jadłem chleb. Fajnie, że jest ktoś kto chce się podzielić swoim punktem widzenia w sprawach, które są przedmiotem moich przemyśleń. Jeszcze raz życzę Ci wszystkiego co najlepsze!

Możliwość komentowania została wyłączona.