Partial update (PATCH) over REST

[bajtos]

Description / Steps to reproduce / Feature proposal

Consider the following acceptance tests for Todo controller in our example apps.

it('updates the todo by ID ', async () => {
  await client
    .patch(`/todos/${persistedTodo.id}`)
    .send({isComplete: true})
    .expect(204);
  const result = await todoRepo.findById(persistedTodo.id);
  const expected = Object.assign(persistedTodo.toJSON(), {isComplete: true};
  expect(result.toJSON()).to.eql(expected);
});

it('returns 404 when updating a todo that does not exist', () => {
  return client
    .patch('/todos/99999')
    .send({isComplete: true})
    .expect(404);
});

Current Behavior

Both tests fail because the PATCH request fails with 422 validation error. The request body is rejected because it's not a valid Todo instance (missing required title property, etc.)

Expected Behavior

The PATCH operation behaves like a real patch, accepting a subset of Todo properties to update. Both tests pass.

Related issues:

• OpenAPI decorator does not properly generate schemas of type Partial OpenAPI decorator does not properly generate schemas of type Partial #1179
• [API Explorer] Fields shown but ignored [API Explorer] Fields shown but ignored #1470

See Reporting Issues for more tips on writing good issues

[raymondfeng]

I'm proposing to introduce an @schema decorator or extend existing decorators so that a schema can be something like:

{
  schema: {
    'x-ts-type': Order,
    'x-ts-type-options': {
      partial: true,
      includes: ['p1', 'p2'],
      excludes: []
    }
  }

[bajtos]

I'm proposing to introduce an @Schema decorator or extend existing decorators so that a schema can be something like.

The discussion about excluding/including certain properties from parameter schema has already started in #1179, let's keep it in a single place there. I'll cross-post your comment @raymondfeng.

IMO, this issue should be focused on leveraging #1179 in our example projects and CLI controller templates.

Alternatively, we can close is as a duplicate of #1179.

[bajtos]

On the second thought, maybe this issue is slightly different from #1179.

In #1179, we want to allow create operation to exclude certain model properties from the payload, for example id.

In this issue, we want to allow all model properties in the payload, but make them all optional.

Ideally, I'd like us to fine a mechanism (decorator API) that can support both use cases in a consistent way.

Building on top of what has been proposed in #1179 (comment):

// `id` property is not allowed for `create`:
@post('/')
createTodo(@requestBody(Todo, {propertyBlacklist: ['id']}) todo: Partial<Todo>) {}

// all properties are allowed, all are optional
@patch('/')
updateTodo(@requestBody(Todo, {partial: true}) todo: Partial<Todo>) {}

Internally, @requestBody can use x-ts-type-options proposed by @raymondfeng as an implementation detail.

[bajtos]

In #2152 (comment), I mention a solution for defining types representing model data with a black-list applied.

• A type used for creating new instances, this type typically excludes properties set by the database (e.g. id, _rev, etc.)

  // black-list based approach
  type NewTodo = Pick<Todo,  Exclude<keyof Product, 'id'>>;
  // white-list base approach
  type BumpCounterAndUpdated = Pick<MyModel, 'counter' | 'updated'>

• A type used for performing a patch (partial update) operation, this type has all properties optional and probably should exclude properties controlled by the database

  type TodoPatch = Partial<NewTodo>;

Now that we know how to deal with types, we need to figure out how to generate OpenAPI schema for these different objects.

[b-admike]

hey @bajtos @raymondfeng, I was trying to compile an acceptance criteria for this issue, but noticed that it's pretty similar to #2652. Can we close it in favour of the other or do you think they're different? If so, maybe we can chat to come up with an acceptance criteria for this issue and get it groomed.

[bajtos]

Let's put this issue on hold until #2652 is done. Quoting from the acceptance criteria from that issue:

Review #1722 and #1179, either close them as fixed or post a comment explaining what's remaining to be done.

[bajtos]

Most of the requested functionality was implemented by #3199. The remaining part is how to simplify @requestBody annotations, I opened a new issue to deal with that: #3257

Closing this one as done.

[odykyi]

user: Partial < User > - works for me

 @patch('/users/{id}', {
    responses: {
      '204': {
        description: 'User PATCH success',
      },
    },
  })
  async updateById(
    @param.path.number('id') id: number,
    @requestBody({
      content: {
        'application/json': {
          schema: getModelSchemaRef(User, {partial: true}),
        },
      },
    })
    user: Partial<User>,
  ): Promise<void> {
    await this.userRepository.updateById(id, user);
  }