citerus / dddsample-core

This is the new home of the original DDD Sample app (previously hosted at sf.net)..
MIT License
4.99k stars 1.47k forks source link

Merge facade and controller for cargoadmin #59

Closed orende closed 1 year ago

orende commented 1 year ago

Merges the facade and controller for the CargoAdmin functionality.

mkocalka commented 1 year ago

Hi Orende, Just for let you know, or at least to evaluate…

Facades, Like BookingServiceFacade, were intended to:

  1. Wrap and fix ORM/PROXY issues,
  2. Provide a portable RMI layer (for old plan java remote interfaces)
  3. BUT more important, at least for me, to provide an ANTI-CORRUPTION layer (in order to not self expose the domain objects/values to web/controllers)

Just in case, Seems to me that Facades are a relevant concept (within ddd-sample context) I would keep them in the sample app or documentation.

In the other hand, Not sure is this is only for peace of mind (just for the sample simplicity) or have changed your mind about facades.

Anyway, Big fan of ddd-sample

Kind regards!

On 18 Oct 2022, at 14:54, orende @.***> wrote:

Merges the facade and controller for the CargoAdmin functionality. Also replaces setter-based injection with ctor-based injection ExternalRoutingService and moves date formatting from the Thymeleaf templates to the backend.

You can view, comment on, or merge this pull request online at:

https://github.com/citerus/dddsample-core/pull/59 https://github.com/citerus/dddsample-core/pull/59 Commit Summary

3e22613 https://github.com/citerus/dddsample-core/pull/59/commits/3e226136c2e9a6b3caac1ba3828edfa42b68f4a8 Merge controller and facade layers for the CargoAdmin pages to simplify things dc7c060 https://github.com/citerus/dddsample-core/pull/59/commits/dc7c06014a0bdf0928597409735f987f712039fb Replace setter-based injection with ctor-based injection for ExternalRoutingService 204ac21 https://github.com/citerus/dddsample-core/pull/59/commits/204ac21827db20714702483c8814d80a0fdf93de Move date formatting from templates to backend File Changes (16 files https://github.com/citerus/dddsample-core/pull/59/files) M src/main/java/se/citerus/dddsample/config/DDDSampleApplicationContext.java https://github.com/citerus/dddsample-core/pull/59/files#diff-02dbc3879492a482971ef3f4921d9910dd61adbf48bd761938c27aef9f30c96c (6) M src/main/java/se/citerus/dddsample/infrastructure/routing/ExternalRoutingService.java https://github.com/citerus/dddsample-core/pull/59/files#diff-4f2d7d6c86a88be22def9bd7dbebf1a392d6283274e8f66f4eb345c7310c362a (27) M src/main/java/se/citerus/dddsample/interfaces/InterfacesApplicationContext.java https://github.com/citerus/dddsample-core/pull/59/files#diff-03b0b35af9fa6f5af3e109ce5f52903c6e03088bcc7cb52ae30c8147d36583ec (11) D src/main/java/se/citerus/dddsample/interfaces/booking/facade/BookingServiceFacade.java https://github.com/citerus/dddsample-core/pull/59/files#diff-0c64b1da34d93740cda89a788dc894bdbaac79effc80961378c97ccf4445c3f7 (31) M src/main/java/se/citerus/dddsample/interfaces/booking/facade/dto/CargoRoutingDTO.java https://github.com/citerus/dddsample-core/pull/59/files#diff-5c1e8db40be786568e6aa35cfee67b7483b31286626fb89549683e44e3b045df (8) M src/main/java/se/citerus/dddsample/interfaces/booking/facade/dto/LegDTO.java https://github.com/citerus/dddsample-core/pull/59/files#diff-473697f436d26f3ae57530bbf32f872e401eab29c39e80554802f7b924f3fa05 (10) D src/main/java/se/citerus/dddsample/interfaces/booking/facade/internal/BookingServiceFacadeImpl.java https://github.com/citerus/dddsample-core/pull/59/files#diff-6ee3bf4bc4a3161647aa7f1af000c2d366640e7e47771c039533bf308b7c145d (108) A src/main/java/se/citerus/dddsample/interfaces/booking/facade/internal/assembler/AssemblerUtils.java https://github.com/citerus/dddsample-core/pull/59/files#diff-a5bad8116f9b073f3376aabfd8ef65c5a2d633780cab6224d8372c954bef1c70 (27) M src/main/java/se/citerus/dddsample/interfaces/booking/facade/internal/assembler/CargoRoutingDTOAssembler.java https://github.com/citerus/dddsample-core/pull/59/files#diff-a1fd467c44d33f6a2962a6f6a07e485aced6d150041a4e36dd4e32854c074af1 (10) M src/main/java/se/citerus/dddsample/interfaces/booking/facade/internal/assembler/ItineraryCandidateDTOAssembler.java https://github.com/citerus/dddsample-core/pull/59/files#diff-13648bb5b0aba36065dcacb9eefed5560fc8c7e4a1925db4b89c73a6da5d7672 (40) M src/main/java/se/citerus/dddsample/interfaces/booking/facade/internal/assembler/LocationDTOAssembler.java https://github.com/citerus/dddsample-core/pull/59/files#diff-218b4160f908f633c2beffe52355c131cab5497f7da0b235b978154941b2a75d (14) M src/main/java/se/citerus/dddsample/interfaces/booking/web/CargoAdminController.java https://github.com/citerus/dddsample-core/pull/59/files#diff-d0b5b9500e8c1e16e81b219a5e3d0400284fdb6ab2a55f63755842cb8ad8e222 (85) M src/main/resources/templates/admin/registrationForm.html https://github.com/citerus/dddsample-core/pull/59/files#diff-f0e121f624e7393678f5f9f9dd40a269caa677765388b6f9399886fb9a34bfa0 (5) M src/main/resources/templates/admin/selectItinerary.html https://github.com/citerus/dddsample-core/pull/59/files#diff-16d9dd9359b58d66c3489c53e48b4ac039c5057dc3bbdfe8664585330f9f62c2 (8) M src/main/resources/templates/admin/show.html https://github.com/citerus/dddsample-core/pull/59/files#diff-a220c20cfb294e3dcaf6ce5fba6da24ae42884d1e82953a0ad89b0023c8ac4de (6) M src/test/java/se/citerus/dddsample/infrastructure/routing/ExternalRoutingServiceTest.java https://github.com/citerus/dddsample-core/pull/59/files#diff-ebf93c99c9057c0ba52b9624efe82624f156f5f74460e42bba632e6917c0d769 (8) Patch Links:

https://github.com/citerus/dddsample-core/pull/59.patch https://github.com/citerus/dddsample-core/pull/59.patch https://github.com/citerus/dddsample-core/pull/59.diff https://github.com/citerus/dddsample-core/pull/59.diff — Reply to this email directly, view it on GitHub https://github.com/citerus/dddsample-core/pull/59, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIQ7TXPPA5HZ5QXTQCFRP6LWD3P7HANCNFSM6AAAAAARIKU5RU. You are receiving this because you are subscribed to this thread.

orende commented 1 year ago

@mkocalka Good points! I have gotten similar feedback on the facade/controller change from other sources as well. Since this change isn't as clearcut as I thought, I'll split this PR into three separate PRs for each commit, link them in this PR and continue the discussion there.

mackapappa commented 1 year ago

After discussion we decided that the facade still serves a purpose, both for the reasons you mentioned and to avoid duplication of gui-logic when implementing another gui.