Code review API Spring Boot

Czołem,
Kiedyś już prosiłem o code review mojego innego projektu i pamiętam, że dało mi to dużo wskazówek do przemyślenia. Dlatego zwracam się do Was o zerknięcie na mój projekt API do zarządzania wydatkami: .

Tutaj sobie to implementuje używając Angulara, więc można się pobawić.

Takie główne pytania:

1. Czy dobrze podzieliłem kod pomiędzy ExpenseService i ExpenseController?
2. Chciałbym dodać użytkowników, tak żeby każdy miał tylko i wyłącznie do swojej bazy wydatków. Stąd pytanie czy dobrze będzie jak do wydatków będzie kierował link users/{user-id}/expenses?
3. Potrzebuję w ogóle podpowiedzi co do Security. Jakim sposobem zabezpieczyć takie pojedyncze bazy danych, jak uwierzytelniać logowanie itp.

Z góry dziękuję za pomoc :)

	public List<Expense> viewExpenses_whenDay(String day) {
		List<Expense> expenses = new ArrayList<>();
		DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd");
		LocalDate date = LocalDate.parse(day, formatter);

		expenseRepository.findAll().forEach(expense -> {
			if (expense.getDate() != null && expense.getDate().compareTo(date) == 0) {
				expenses.add(expense);
			}
		});
		return expenses;
	}

Takie rzeczy robi się w kwerendzie. W Spring Data masz adnotację @Query.

W ExpenseRepository:

@Query("from Expense where date = :day")
List<Expense> findByDay(@Param("day") LocalDate day);

Potem w ExpenseService po prostu używasz tej metody.

1. Nie używaj podkreślników przy nazwie metody. Jak masz w argumencie day to już nie musisz tego uwzględniać w nazwie metody. Zamień też String na LocalDateTime.

public List<Expense> viewExpenses_whenDay(String day)

Mógłbyś tę metodę nazwać

public List<Expense> getExpenses(LocalDateTime day)

1. Możesz skorzystać java8 Stream API. Pozbędziesz się niepotrzebnej zmiennej expenses

List<Expense> expenses = new ArrayList<>();
expenseRepository.findAll().forEach(expense -> {
			if (expense.getDate() != null && expense.getDate().compareTo(date) == 0) {
				expenses.add(expense);
			}
		});
return expenses;

zamień na

return expenseRepository.findAll()
                         .stream()
                         .filter(expense.getDate() != null)
                         .filter(expense.getDate().compareTo(date) == 0)
                         .collect(toList());

1. Wydatki to nie expences tylko expenses. Gdzieniegdzie masz literówki.
2. Po co ci ten plik system.properties. Jak masz zmienne to trzymaj je w application.properties

@RestController
@RequestMapping("/categories")

możesz zamienić na

@RestController(value="/categories")

1. Używaj wstrzykiwania zależności przez konstruktor.

public class CategoryController
    {
        @Autowired
        private CategoryRepository categoryRepository;
    }

zamień na

public class CategoryController
    {
        private final CategoryRepository categoryRepository;
    
        CategoryController(categoryRepository CategoryRepository)
        {
            this.categoryRepository = categoryRepository;
        }
    }