Skip to content

Refactor Remote Dev to a single common service class

Chad Woolley requested to merge caw-rd-remove-service-layer into master

Issue: Minimize/DRY Service Layer in Remote Developmen... (#452476 - closed)

What does this MR do and why?

  1. Refactors Remote Dev domain to use a single common service class
  2. Other related cleanup/refactors
  3. Relevant updates to related docs and READMEs

See the issue at Minimize/DRY Service Layer in Remote Developmen... (#452476 - closed) for the background context on this decision.

Details

Because of the Layered Architecture used by the Remote Development bounded context, there is intentionally no use-case-specific logic in the Service Layer.

Therefore, we have refactored the Service Layer for this domain to be a single reusable CommonService class.

This allows the following benefits for the Service Layer:

  1. It is DRY with minimal repetition of logic across multiple service classes
  2. Multiple specs do not need to be written, run, and maintained for multiple service classes
  3. The behavior of the service layer is cohesive - it only has a limited set of specific and explicit responsibilities:
    1. Accept the arguments passed from the API layer, and pass them to the correct Main class in the Domain Logic layer.
    2. Inject additional dependencies, such as Remote Development Settings and logger, into the Domain Logic layer.
    3. Convert the "response_hash" return value from the Domain Logic layer into a ServiceResponse object.
    4. Enforce at runtime the Functional Patterns used within the domain.

On the last point, we have updated the domain documentation to be explicit about our decision to enforce the functional patterns we are using at runtime.

This MR also updates the development guidelines for Service Classes to make it explicit that while this MR introduces a nonstandard approach for calling the Remote Development Workspaces Service Layer, the contract for the return value is still respected and consistent with the requirement that Service Classes should return a ServiceResponse object

Acceptance Criteria

  • Refactor the service tier to a single class.
  • Add additional support for enforcement of patterns used in the Domain Layer of the Remote Development bounded context
    • Better error message for pattern violation - give guidance on how to make class method private
    • Move extended modules' private singleton declarations into module itself
  • Update the ee/lib/remote_development/README.rb to describe new Service Layer approach with examples
  • Update the ee/lib/remote_development/README.rb to be explicit about patterns, including clarification on decision to enforce patterns at runtime, and not just the linter or spec level.
  • Update docs.gitlab.com regarding Service Class interface and contracts

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Chad Woolley

Merge request reports