Addition of brands and models not contained in the DB.

[thrako]

Лека доработка, за да не дискриминираме собствениците на Ferarri, Trabant, Volkswagen и други марки автомобили, които към момента не се съдържат в базата данни :)

• В html формата за добавяне на оферта е добавено поле Brand. За да се запазят подсказките в Brand и Model полетата, <select> елементът е заменен с <datalist>, който се попълва с <option> елементи с JS, а информацията се изтегля през REST . Валидационните съобщения се визиуализират под всяко поле, а самите те са изнесени във ValidationMessages.properties, за да са готови за интернационализиране.
• Добавянето на оферта минава през OfferProcessor (не успях да измисля по-добро име), което е композитен service клас, държащ в себе си няколко service-а. Мисля, че това е необходимо, защото се прави проверка дали вече не се пазят същите BrandEntity и/или ModelEntity в базата данни, в противен случай се създават нови, а не ми се струва редно това да се прави в OfferService-а. Избягвам в service класовете да добавям като dependency друг service, заради опасността от circular dependency.
• Връзката между BrandEntity и ModelEntity е върната на unidirectional; N+1 проблемът е адресиран с ad hoc @EntityGraph в ModelRepository. По-нататък, ако се наложи да имаме bidirectional връзка, ще я добавим.

Поздрави,
Траян

[luchob]

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

[thrako]

Отговорът ми е под въпроса относно circular dependency, т.к. са свързани.

[luchob]

Това за circular dependency-то не го разбрах много добре. По какъв начин ще го избегнем с клас, който се казва Processor вместо сървис, може ли премер?

[thrako]

Ще започна по-отдалеч: след изменението във формата за добавяне на оферти, вече имаме две хипотези:

• избраната марка (BrandEntity) и/или модел (ModelEntity) вече съществуват в базата данни; или
• добавена е нова марка и/или модел => трябва да създададем ново BrandEntity и/или ModelEntity и да запазим в базата данни.

За да разберем в коя хипотеза се намираме трябва да:
(А) направим проверка в съответното repository (Brand/Model); и евентуално
(Б) да създадем ново entity (Brand/Model), което да запазим в базата данни - пак през съответното repository.

Възможните подходи, за които се сещам (допускам, че съществуват други, по-добри) са следните:

1. OfferController държи OfferService, което на свой ред държи: OfferRepository, BrandRepository и ModelRepository.
(относно "А") Не виждам нищо нередно в това OfferService да проверява наличието на entity като извика съответния метод на repository-то; но
(относно "Б") Някак не ми се струва редно OfferService да създава ново BrandEntity или ModelEntity и да го запазва в базата данни. Тази работа си е присъща на BrandService или съответно ModelService. Ако от друго място в кода ми се наложи да създавам и записвам ModelEntity например, първо бих потърсил в ModelService дали вече няма такъв метод и ако няма да го създам (=> дублираме код), не бих предположим, че в OfferService вече има такава функционалност, а дори и да зная пак би изглеждало странно да викам OfferService от друго място в кода, което няма общо с офертите. Това ни води до следващия подход...

2. OfferController държи OfferService, което на свой ред държи: OfferRepository, BrandService и ModelService.
(относно "А") На OfferService му е необходимо ModelEntity (съществуващо или новосъздадено), за да запази нова оферта. Ако си го взема през ModelService, то това означава да връщаме entity от service метод (за което си ми обърнал внимание по-горе). Това само по себе си още не е толкова лошо, ще стане наистина лошо, ако си позволяваме да работим с entity-та в controller.
(относно "Б") Създаването и записването на BrandEntity си е присъщо за BrandService, проблемът от 1(Б) изглежда решен, но това отваря друг проблем. Ако предприемем подход, в който service-ите, могат да depend-ват един на друг, вероятността, особено при наличието на bi-directional related entities, да се получи circular dependency е голяма (виждал съм го в проекти на няколко колеги). В случай че не използвам правилния термин, то под "circular dependency" имам предвид следното: ModelService държи BrandService и едновременно с това BrandService държи ModelService и при създаването на bean-овете Spring изпада в ситуация тип Paragraph 22.

3. OfferController държи OfferProcessor (може да държи други processor-и, но не и service-и); OfferProcessor на свой ред държи OfferService, BrandService, ModelService и всякакъв друг service, който може да му потрябва, за да свърши работата, възложена му от OfferController.
(относно "А") Ако всеки controller си комунира само с processor-и (ах, как не се сещам за по-добро име), то вече няма проблем методите на service-ите да връщат entity, когато се налага. Забележи, че OfferProcessor връща UUID към OfferController, а не entity.
(относно "Б") Проблемът със circular dependency между service-те няма как да възникне. Всеки service върши работата, която му е присъща.
Да, увеличаме сложността като въвеждаме още едно ниво (Controller -> Processor -> Service -> Repository), което е цената, която плащаме, за да решим разгледаните по-горе проблеми. Вероятно има по-добър начин, просто аз не се сещам за такъв.

[luchob]

Чудесно, таман може би ще го мърджнем след лекцията за РЕСТ.

[luchob]

Привет, само набързо я видях тъй като не съм в софия. Другата седмица се прибирам и ми се иска да я разгледам на спокойствие, т.к. е доста обемна.

[thrako]

Привет, само набързо я видях тъй като не съм в софия. Другата седмица се прибирам и ми се иска да я разгледам на спокойствие, т.к. е доста обемна.

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

С пожелание за хубав ден,
Траян

[luchob]

Привет!

Честно казано, подхода с процесора не ми допада нито съм работил по подобен начин. Ще се опитам да обясня по-простичко каква алтернатива имам предвид.

Разглеждам сървиса като нещо от сорта на repository + бизнес логика. Само че не разглеждам репозиторито като мапинг между ентити и таблица в ДБ. По-скоро гледам на него като на колекция от някакви ентитита (модел, бранд и т.н.). Или на практика би могло да са няколко спринг репозиторита. (OfferService с offerRepo, modelRepo, brandRepo). Не виждам нищо нередно да се дърпа ентити което не принадлежи към сървиса (например модел от офър сървис).

По-скоро както си казал е неприятно ако в сървиса се появи бизнес логика чието място не е там. Например създаване на бранд или модел в офър сървис. Това не е ок и мястото на тази логика е в brand/model service.

Затова по скоро си представям нещата като:

OfferService {
	
	createOffer() {
		brandRef = brandService.createBrand(brandDTO);
		modelRef = modelService.createModel(brandDTO, modelDTO);

		modelEntity = modelRepo.findModelBy..(modelRef)
		...
		offer.setModel(..)
	}
}

Иначе си прав, че е възможно да се появи някакъв неприятен circular dependency. Опитвам с нещо простичко, което сега ми дойде наум като пример:

@Service
class UserService {
	
	OrderService orderService;

	boolean isPremiumUser(UserDTO user) {
		// business logic, which user is premium so that we can offer some 
		// fancy stuff around the shop?
		return orderService.getFinishedOrderCount(user) > 10;
	}
}

@Service
class OrderService {

	OrderRepository orderRepo;
	UserService userService;
	
	int getFinishedOrderCount(UserDTO userDTO) {
		// business logic - which order is finished? Maybe one with confirmed delivery and no complaints in the first 10 days.
		return orderRepo.countOrdersByUser(..).stream().filter(/*business logic*/).count();
	}

	int getOrderDiscount(OrderDTO order, UserDTO userDto) {
		// discount is greater for premium users and expensive orders
		return order.value() > 100 && userService.isPremiumUser(userDto) ? 10 : 2;
	}
}

Тук имаме

      	 ----------------------------
     	|					  		 |			
     OrderService <-----UserService<-

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

@Service
class UserService {
	
	FinishedOrderService finishedOrderService;

	boolean isPremiumUser(UserDTO user) {
		return finishedOrderService.getFinishedOrderCount(user) > 10;
	}
}

@Service
class OrderService {

	OrderRepository orderRepo;
	UserService userService;
	

	int getDiscount(OrderDTO order, UserDTO userDto) {
		return order.value() > 100 && 
			userService.isPremiumUser(userDto) ? 10 : 2;
	}

	...
}

@Service
class FinishedOrderService {

	OrderRepository repository;
	
	int getFinishedOrderCount(UserDTO userDTO) {
		return orderRepo.getOrdersByUser(..).stream().filter(...).count();
	}

}

[thrako]

Здравей,

Честно казано, подхода с процесора не ми допада нито съм работил по подобен начин.

И на мен не ми допада, особено името, но за момента не съм намерил нищо по-добро (нито като дизайн, нито като име). Виж каква дискусия са си спретнали тук (с участието на Andy Wilkinson):
spring-projects/spring-boot#27652 (comment)
Общо взето същото като решението, до което аз стигнах като дизайн, само че вместо processor си го наричат service с по-дълго име (комбинация от двата сървиса), което в нашия случай трябва да е нещо като OfferModelBrandService (още по-грозно от processor). Другият вариант е, ако се добавя ниво в йерархията, то да се добави DAO ниво между service & repository, така DAO-то може да вземе част от функциите на service-а и няма пречка един service да има няколко DAO-та. На практика е същото, но са наименовани по друг начин (controller --(to many)--> service --(to many)--> DAO --(to one)--> repo).

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

@Service
class UserService {
	
	FinishedOrderService finishedOrderService;

	boolean isPremiumUser(UserDTO user) {
		return finishedOrderService.getFinishedOrderCount(user) > 10;
	}
}

@Service
class OrderService {

	OrderRepository orderRepo;
	UserService userService;
	

	int getDiscount(OrderDTO order, UserDTO userDto) {
		return order.value() > 100 && 
			userService.isPremiumUser(userDto) ? 10 : 2;
	}

	...
}

@Service
class FinishedOrderService {

	OrderRepository repository;
	
	int getFinishedOrderCount(UserDTO userDTO) {
		return orderRepo.getOrdersByUser(..).stream().filter(...).count();
	}

}

Моля да ме извиниш за думите, но това решение не ми изглежда съвсем чисто. За всяка следваща функционалност, отнасяща се до Order, като я разписвам, ще трябва да се чудя в кой от двата service-а да я сложа (finished или unfinished). Още по-лошо, след това като ми трябва да ползвам тази функционалност, ще се чудя в кой сървис беше.

Попрочетох из мрежата за проблема: масово се коментира, че circular dependency възниква от bad design, но никой не казва какво разбира под good design to avoid it (с изключение на предложението от линка по-горе, както и event driven design, което според мен би било way too much да имплементираме в рамките на този курс). В масовия случай се предлага lazy instantiation (с @Lazy @Autowired) или Setter Injection и подобни бързи и мръсни workaround-и.

Може би към момента изглежда, че се опитвам да реша несъществуващ проблем, но ако дизайнът ни предполага един service да депендва на друг service - рано или късно ще стигнем до circular dependency, ако не в този проект, то в друг. И тогава, когато е разписан много много код, няма да има как да изменим дизайна безболезнено - ще трябва да използваме някой мръсен трик, за да работи. Продължавам да мисля по въпроса :-)

Поздрави,
Траян

[thrako]

Здравей отново,

Една идея, която ме озари малко по-късно. Ако се водим от принципите на Domain Driven Design и следната класификация: https://martinfowler.com/bliki/EvansClassification.html , то domain entity-та, които нашето приложение има, общо взето се ограничават до Offer, User и Admin. Model и Brand в нашето приложение са по-скоро Value Object в контекста на класификацията по-горе. Оттук ми изниква въпроса, необходимо ли е изобщо да имаме ModelService и BrandService? Какво ще правят тези service-и, което е различно от заявки към базата данни? Оттам и екстремистката идея да ги елиминираме изцяло, а цялата логика, която се отнася до Offer, Brand и Model да си остане в OfferService. Ти какво мислиш за тази екстремистка идея? :-)

Поздрави,
Траян

[luchob]

Привет!

Ама това изобщо не е екстремистка идея :-) Напротив, много често се "замонолитват" така в по големи проекти. Мен ако питаш засега имаме само Offer и User, но мисля и да добавя currency, може би вече като нещо по-отделно. Искам да се интегрираме с open exchange rates за да покажем REST API и цените да може да се виждат и в EUR.

Иначе и ние си правим организацията на проектите в отделни пакети за нещо което считаме че е домейн. Ще ти покажа отделно такава структура.

Ще вкараме неща оттук в проекта, но не мога да я мърджна вътре защото ще стане твърде променено.

Поздрави,
Л.

[vando1661]

Привет Лъчо и честит имен ден на патерици. Искам да попитам ако имаме повече от admin и user роли примерно seller и неговото Entity е различно като модел от това на обикновения user и admin как ще ги регистрираме от еднин и същ бутон register. Или трябва др ботун с определената форма. Надявам се ,че съм обяснил правилно. Лек ден.

[luchob]

Привет Лъчо и честит имен ден на патерици. Искам да попитам ако имаме повече от admin и user роли примерно seller и неговото Entity е различно като модел от това на обикновения user и admin как ще ги регистрираме от еднин и същ бутон register. Или трябва др ботун с определената форма. Надявам се ,че съм обяснил правилно. Лек ден.

Здравей, може би за такъв въпрос щеше да е подходящо ново issue. Но както и да е. :-)

Ако се налага регистрация за различни видове юзъри, бих заложил съответно на различни форми + фрагменти за повтавяемата част.

Например, цъкаш регистър и си избираш какво ще регистрираш - seller, buyer, каквото и да е. Оттам те пренасочва към специфичната форма.

Съответно имаш и специфичен ендпойнт в контролера за всеки тип, където са ти и валидациите.

В thymeleaf повтаряемата част се изнася във фрагменти, а пък повтаряемата част в кода на сървъра би било редно да се изнесе в отделни методи на сървиса или пък дори в отделни сървиси.

Поздрави, Л.

[vando1661]

Извиням се не съобразих. Благодаря много за отговора. И аз така си мислех с различни форми ,но като изпозваме един бутон регистрация така няма ли опасност някой user да има възможност и желание да се регистрира като admin.
Поздрави Иван.

[luchob]

Извиням се не съобразих. Благодаря много за отговора. И аз така си мислех с различни форми ,но като изпозваме един бутон регистрация така няма ли опасност някой user да има възможност и желание да се регистрира като admin. Поздрави Иван.

Разбира се, един анонимен юзър не би трябвало да може да се регистрира като админ :-) Това е абсурдно. Как да се направи е въпрос не толкоз технически. Но например, би могло да е възможно само един админ да направи нормален юзър админ с помощта на някакво UI. По същия начин както ти би могъл да ми дадеш право да пушвам в твое репозитори в GitHub.

Поздрави,
Л.