Skip to content
This repository has been archived by the owner on Sep 7, 2021. It is now read-only.

Feature/input dtos #11

Closed
wants to merge 1 commit into from
Closed

Feature/input dtos #11

wants to merge 1 commit into from

Conversation

jhoogstraat
Copy link
Contributor

@jhoogstraat jhoogstraat commented Apr 7, 2020

Provide a way to define the layout of request bodies and how the data is processed.

Description

Adds two Modifiers (SaveDTOModifier and UpdateDTOModifier) which require the programmer to define a struct conforming to either SaveDTOor UpdateDTO. These types are then decoded from the request body and used to save or update a model to the database.

Ideas

  • Use Validations if present
  • use OperationType .patch for updating, because of its 'partial updating' nature.

Related Issue

Fixes 50% of #3

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • SwiftLint throws no warnings or errors.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@jhoogstraat
Copy link
Contributor Author

I’d like to emphasize that this solution does not allow the DTOs to grab information from other parts of the request than the request body (e.g. path components, query string or headers) in a very type safe manner. It is recommended to require requests to provide all information necessary to create the database item in the request body. That being said the DTOs still have access to the Request object, so no hard limitations there.

Copy link
Contributor

@bmikaili bmikaili left a comment

Choose a reason for hiding this comment

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

Looks nice overall! Just some questions for my understanding left as comments.

Tests/CorvusTests/ApplicationTests.swift Outdated Show resolved Hide resolved
Tests/CorvusTests/Utilities/JSON.swift Outdated Show resolved Hide resolved
@bmikaili bmikaili requested a review from PSchmiedmayer April 13, 2020 14:02
@bmikaili bmikaili added the enhancement New feature or request label Apr 13, 2020
@bmikaili bmikaili linked an issue Apr 13, 2020 that may be closed by this pull request
@jhoogstraat
Copy link
Contributor Author

Before we merge, i think it should be discussed if this solution provides enough flexibility and performance (uses Mirror multiple times currently). I am currently working on supporting Enums and Parent/Sibling relations.

@bmikaili
Copy link
Contributor

I think it's fine, performance isn't that big a priority, and it's flexible enough for what it needs to do.

@bmikaili
Copy link
Contributor

bmikaili commented Apr 17, 2020

@jhoogstraat Can you merge develop into this, then I would approve and merge the PR. You can use your CreateEndpoint, the implementations in develop only require that the protocol exists.

@jhoogstraat
Copy link
Contributor Author

jhoogstraat commented Apr 18, 2020

Changes are currently untested. Feedback welcome.

@PSchmiedmayer
Copy link
Contributor

We will archive Corvus in favor of Apodini as a worthy successor.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Implement Mediator types
3 participants