feat: apply partial updates and concurrent guard

[minottic]

Description

Add support for merge-patch and unmodified since to policy

Tests included

• Included for each change/fix?
• Passing?

Documentation

• swagger documentation updated (required for API changes)
• official documentation updated

official documentation info

Summary by Sourcery

Enable merge-patch policy updates and protect conditional writes with optimistic concurrency control.

New Features:

• Support partial policy updates using merge-patch semantics, including explicit null resets and nested object merging.
• Add optimistic concurrency control for policy updates through the If-Unmodified-Since header, returning HTTP 412 for stale concurrent writes.

Enhancements:

• Document the supported policy patch media types and partial-update behavior in the v4 API specification.

Tests:

• Add unit and integration coverage for merge-patch null handling, nested updates, stale timestamps, and concurrent policy updates.

[sourcery-ai]

Hey - I've found 4 issues

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="src/policies/policies.v4.controller.ts" line_range="129-130" />
<code_context>
     @Param("id") id: string,
     @Body() updatePolicyDto: UpdatePolicyV4Dto,
   ): Promise<Policy | null> {
+    const isMergePatch =
+      request.headers["content-type"] === "application/merge-patch+json";
+    const unmodifiedSince = parseDate(request.headers["if-unmodified-since"]);
+
</code_context>
<issue_to_address>
**issue (bug_risk):** `application/merge-patch+json` requests with a charset parameter, such as `application/merge-patch+json; charset=UTF-8`, are treated as ordinary JSON because the raw content type is compared for exact equality. Null properties are therefore ignored instead of being reset as merge-patch requires.

**Triggers:** When a client includes a charset parameter in the Content-Type header.

**Suggested fix:** Parse the media type before comparing it, or use the framework's normalized content-type value.

```suggestion
    const isMergePatch =
      request.headers["content-type"]?.split(";")[0].trim() ===
      "application/merge-patch+json";
```
</issue_to_address>

### Comment 2
<location path="src/policies/policies.service.ts" line_range="176-183" />
<code_context>
+    setFields.updatedAt = new Date();

-    return this.policyModel
+    const queryFilter = withOCCFilter(filter, unmodifiedSince);
+    const updated = await this.policyModel
       .findOneAndUpdate(
-        filter,
+        queryFilter,
         { $set: setFields },
         { new: true, runValidators: true },
       )
       .exec();
+
+    if (!updated && unmodifiedSince) {
</code_context>
<issue_to_address>
**issue (bug_risk):** The OCC check uses `$lte` against a timestamp and then writes a new timestamp, so two concurrent updates whose initial `updatedAt` and generated update times fall in the same millisecond both match the filter and succeed. The concurrent guard therefore does not guarantee that one request receives HTTP 412.

**Triggers:** When concurrent updates are processed within the same millisecond.

**Suggested fix:** Use an atomic version field increment, or otherwise ensure every successful update changes the OCC value before another matching update can proceed.
</issue_to_address>

### Comment 3
<location path="src/policies/policies.service.ts" line_range="176" />
<code_context>
+    setFields.updatedAt = new Date();

-    return this.policyModel
+    const queryFilter = withOCCFilter(filter, unmodifiedSince);
+    const updated = await this.policyModel
       .findOneAndUpdate(
-        filter,
+        queryFilter,
         { $set: setFields },
         { new: true, runValidators: true },
       )
       .exec();
+
+    if (!updated && unmodifiedSince) {
</code_context>
<issue_to_address>
**issue (bug_risk):** An `If-Unmodified-Since` value expressed as an HTTP-date has only second precision, while `updatedAt` is stored with millisecond precision; the `$lte` comparison rejects an unchanged policy whenever its stored timestamp has milliseconds later than the header's rounded-down second.

**Triggers:** When a client sends the standard second-precision HTTP-date form of If-Unmodified-Since.

**Suggested fix:** Compare timestamps at HTTP-date precision, or use a strong version/ETag-based concurrency token instead.

```suggestion
    const queryFilter = withOCCFilter(
      filter,
      unmodifiedSince && new Date(unmodifiedSince.getTime() + 999),
    );
```
</issue_to_address>

### Comment 4
<location path="src/policies/policies.service.ts" line_range="168-172" />
<code_context>
+    const flattened = this.flattenToDotPaths(updatePolicyDto);
+    // application/json: a null value means "do not change this field".
+    // application/merge-patch+json: a null value means "reset this field to null".
+    const setFields = Object.fromEntries(
+      Object.entries(flattened).filter(
+        ([, value]) => isMergePatch || value !== null,
+      ),
+    );
+    setFields.updatedBy = username;
+    setFields.updatedAt = new Date();
</code_context>
<issue_to_address>
**issue (bug_risk):** Merge-patch requests that set the required `isPublished` field to `null` are sent as `$set: { isPublished: null }`; Mongoose validation rejects this required field, so the request fails instead of resetting it to null or its default as the endpoint documentation promises.

**Triggers:** When an application/merge-patch+json request contains `isPublished: null`.

**Suggested fix:** Define the supported null/reset behavior for required fields and apply the schema default or reject the operation with a documented client error before issuing the update.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[minottic]

this will need a change after #2929 is merged