Проверка на проект

[magdalenavelikova]

Връзка към проекта:

Поставете [връзка](https://github.com/) към проекта си тук.

Здравейте, знам, че отнема време, което ви е ценно, но ви моля, когато имате възможност да погледнете проекта ми и да ми кажете какво мислите.

P.S. Остават ми няколко функиционалности като родословно дърво и добавяне на титли от съзтезания. И тестовете разбира се :).

Благодаря Ви предварително!

[luchob]

Здравей, действително би отнело прилично количество време да го разгледам ред по ред :-)
Но все пак минах през повечето код и отдолу ще изредя някои впечатления и препоръки.

• огромен проект, значително повече отколкото се иска :-) браво!
• като пакетна структура ми изглежда много подреден, съвсем леко може да извадиш конфигурацията от core, т.к. не бих се сетил да я търся там, има 1-2 класа като ValidationProcessor, FieldError, AppException, UserRoleService и т.н. дето не са на място или са странни :-) . Но нищо шокиращо.
• относно listeners - аз лично бих предпочел @EventListenerметод в съответния сървис, но това е просто предпочитание и не е необходимо да се съобразяваш с него
• странен ми е респонс статуса на някои ексепшъни - @ResponseStatus(value = HttpStatus.FORBIDDEN, reason = "Email/Username is not unique."). Защо пък forbidden? Конкретния повече ми прилича на conflict. Форбидън означава, че нямаш разрешение да направиш нещо - напр. да изтриеш куче.
• това изобщо не ми харесва, дори мисля че е грешно - представи си паралелна регистрация от два потребителя, например или генерация на ид-та с ид генератор вместо ДБ autoincrement:

        if (parentDto.getRegistrationNum().isEmpty()) {
            long id = dogRepository.findFirstByOrderByIdDesc().getId() + 1L;
            parentDto.setRegistrationNum("Parent" + id);
        }

• внимавай с eager релациите (напр DogEntity ако има голямо родословно дърво :-) ). Опитвай се да ги оптимизираш с lazy релации или ентити граф (за което говорихме веднъж)
• в ентититата може да използваш enums, когато е възможно. Например:

    @Column(nullable = false)
    private String sex;

Това може да го замениш с enum Sex вместо String с безброй възможни стойности.

• @Scheduled взема и тайм зона, ако искаш да е наистина в определен час вместо UTC което не взема предвид тайм зоните
• differenceService ми изглежда прекалено ексцентричен заместител на equals за мен :-) Бих го изтрил :-) Още по-странно е че го използваш като булева променлива.
• разгледай кода и провери за някакви такива неща:

...
 if (status(e.getExpiryDate()).equals("upcoming"))
...


    private static String status(LocalDate expireDate) {

        if (LocalDate.now().isAfter(expireDate)) {
            return "passed";
        } else {
            return "upcoming";

        }
    }

не е ли по-лесно просто:

  if (!LocalDate.now().isAfter(e.getExpiryDate()))

Или:

          boolean valid;
        if (firstValue == null) {
            valid = secondValue == null;
        } else {
            valid = firstValue.equals(secondValue);
        }

Може да стане:

          boolean valid = Objects.equals(firstValue, secondValue);

• контролерите са thin, това е добре, правилно структурирани. Може би подобен код би могъл да е в сървиса:

        if (user.getAuthorities().contains(new SimpleGrantedAuthority("ROLE_MEMBER"))) {
            eventPublisher.publishEvent(new OnDogRegistrationCompleteEvent(this, saved.getRegistrationNum(),
                    request.getLocale()));
        }

Поздрави,
Л.

[magdalenavelikova]

Здравейте, наистина много ви благодаря за отделеното време и градивните критики.

Относно “има 1-2 класа като ValidationProcessor, FieldError, AppException, UserRoleService" . UserRoleService никога не е използван. В началото мислех, чрез този сървиз да взимам всички роли и така да ги валидирам на front end - а. Естествено после разбрах, че мога да си декодирам токена и съм забравила да го изтрия. FieldError, AppException преместих в model/exception, а ValidationProcessor в validation.

" if (parentDto.getRegistrationNum().isEmpty()) {
long id = dogRepository.findFirstByOrderByIdDesc().getId() + 1L;
parentDto.setRegistrationNum("Parent" + id);
}"
замених с
UUID uuid = UUID.randomUUID();
String uniqueID = uuid.toString();

Преместих EventListener методите
Оправих ResponseStatus - ите
и продължавам, за да изчистя всички грешки и направя кода си по-малко инфантилен.

И ако идвате във Варна и ви е много скучно пишете. Имам да черпя :)

[luchob]

А, да - и внимателно с употребата на @ в github.

Защото... @eventlistener reacted with 😕 хахаха :-)

[magdalenavelikova]

Здравейте, отново имам нужда от малко помощ. В проекта задължително трябва да има Interceptor и понеже нищо не мога да измисля, видях, че в миналото издание давате идея за IpBlockInterceptor и ми хрумна, че би било чудесно да направя същото за руски IP-та и да им връщам едно json -че {"message": "Чай за Путин!"}, обаче как точно бих могла да прихвана IP-та по гео локация, защото от тук https://lite.ip2location.com/russian-federation-ip-address-ranges?lang=en_US, разбирам, че Russian Federation has a total of 45,358,848 IP address assigned и наистина не знам, как да подходя.

[luchob]

🇺🇦 :-) Доста амбициозно и честно казано мисля, че е много трудно да се направи. Въпреки това, IP адресите се раздават от IANA и има 5 подорганизации, които раздават регионално адреси - RIPE NCC e нашата. Тук можеш да свалиш някакъв списък правила, разделени по страни -> https://www.ip2location.com/free/visitor-blocker . Например има .htaccess което е блок лист за apache httpd. Само че, понякога от т.нар. руски адреси се дават на не руски страни и... абе май е доста сложно :-) и по-скоро се прави на по-ниско ниво (load balancer или някакво nginx proxy и т.н. Никога не съм правил подобно нещо.