Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Separate object models into domain model and DTO #2702

Merged
merged 15 commits into from
Jan 31, 2022

Conversation

apanicker-nflx
Copy link
Collaborator

@apanicker-nflx apanicker-nflx commented Jan 17, 2022

Pull Request type

  • Other (please describe):
    Separate object models into Data Transfer Object and Domain Object

Changes in this PR

A new object model is introduced that will be used by the state machine and all internal processes within Conductor. All external facing APIs and client will continue to use the existing object model. A new class ModelMapper is introduced to provide the translation between the domain object and the data transfer object.

The separation of domain object from the data transfer object completely decouples the conductor state machine from the user-facing APIs. Some internal fields that are only used by the state machine (e.g. executed, subWorkflowChanged etc) are currently exposed to the users because the same object model is used by the state machine and the DTO layer. This separation allows the evolution of the state machine and features therewith, without impacting the user. It also helps abstract away any of the state machine complexities while providing a clean and lean object model for the user to work with.

Additionally, because of the way the object models are intertwined currently, payloads stored in external storage need to be manipulated (uploaded/downloaded) depending on the call stack, either from state machine or api layer, so that large payloads are not sent over the wire and are only available for use within the state machine. The separation of object models enables the state machine to download the payload from external payload storage once during a given workflow evaluation without needing to examine the call stack. The API layer does not need to deal with the large payloads since the model mapper handles the mapping to the DTO.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 17, 2022

Unit Test Results

   164 files  +1     164 suites  +1   10m 3s ⏱️ + 1m 10s
1 145 tests +6  1 145 ✔️ +6  0 💤 ±0  0 ±0 

Results for commit 9830fca. ± Comparison against base commit c0513c0.

♻️ This comment has been updated with latest results.

@apanicker-nflx apanicker-nflx added the type: important Important changes label Jan 19, 2022
@indiv0
Copy link

indiv0 commented Jan 19, 2022

What's the driving factor behind this change, if you don't mind my asking? DTO/DO separation tends to add a lot of complexity in my experience.

@apanicker-nflx apanicker-nflx force-pushed the object_model_separation branch 2 times, most recently from f5ca4f7 to 916a52f Compare January 27, 2022 22:09
@apanicker-nflx apanicker-nflx marked this pull request as ready for review January 27, 2022 22:10
@apanicker-nflx apanicker-nflx force-pushed the object_model_separation branch from 916a52f to 9830fca Compare January 27, 2022 22:26
@apanicker-nflx
Copy link
Collaborator Author

What's the driving factor behind this change, if you don't mind my asking? DTO/DO separation tends to add a lot of complexity in my experience.

@indiv0 While it does add some complexity at the beginning, it completely decouples the conductor state machine from the user-facing APIs. Some of the internal fields used by the state machine (eg. executed, subWorkflowChanged etc) are currently exposed to the users because the same object model is used by the state machine and the DTO layer. This separation allows the evolution of the state machine and features therewith, without impacting the user.

@@ -184,3 +184,14 @@ configure(allprojects - project(':conductor-grpc')) {
}
}
}

["cassandra-persistence", "core", "redis-concurrency-limit", "test-harness"].each {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this configuration be done for all projects? We will end up adding Spock tests for most of the modules.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can extend this to all java projects, i only added the ones that currently have spock tests

@jxu-nflx jxu-nflx self-requested a review January 31, 2022 22:04
@apanicker-nflx apanicker-nflx merged commit cd528e1 into main Jan 31, 2022
@apanicker-nflx apanicker-nflx deleted the object_model_separation branch January 31, 2022 23:46
@apanicker-nflx apanicker-nflx changed the title Object model separation Separate object models into domain model and DTO Feb 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: important Important changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants