Skip to content

[csharp][generichost] Refactor AsModel template - #24650

Merged
wing328 merged 1 commit into
OpenAPITools:masterfrom
devhl-labs:devhl/refactor-AsModel-template
Aug 9, 2026
Merged

[csharp][generichost] Refactor AsModel template#24650
wing328 merged 1 commit into
OpenAPITools:masterfrom
devhl-labs:devhl/refactor-AsModel-template

Conversation

@devhl-labs

@devhl-labs devhl-labs commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

[csharp][generichost] Add OnXxx partial hook to response deserialization methods

Problem

The only supported customization point for response deserialization in the generichost library is the AsModel.mustache template. Overriding that template replaces the body of every Ok() method (and its siblings for other HTTP status codes) across every generated ApiResponse class. There is no way to target a single endpoint or a single response type — the override is all-or-nothing.

This forces library owners into an awkward position: either accept the default deserialization everywhere, or take ownership of a template that must handle all cases, duplicating the default logic for the common path and layering special cases on top of it.

The existing AfterXxx partial hook runs after the full API call and receives the ApiResponse object, but modifications to the deserialized result made there are discarded: the next caller of Ok() triggers a fresh deserialization from RawContent, losing any patches.

There was no supported way to intercept or augment deserialization for a specific response class without overriding the template globally.

Solution

Split the generated Ok() (and equivalent methods for every non-Ok HTTP status code) into three parts:

// generated after this change
public Widget? Ok()
{
    bool suppressDefault = false;
    Widget? result = null;
    OnOk(ref suppressDefault, ref result);
    if (!suppressDefault)
        result = DefaultOk();
    return result;
}

private Widget? DefaultOk()
{
    return IsSuccessStatusCode
        ? JsonSerializer.Deserialize<Widget>(RawContent, _jsonSerializerOptions)
        : default;
}

partial void OnOk(ref bool suppressDefault, ref Widget? result);

The partial void OnXxx method follows the same suppress-default pattern already used by AfterXxx and OnErrorXxx in the outer API class. Library owners implement it in a partial class for only the specific response classes they need to customize:

// library owner code — not generated
public partial class GetWidgetApiResponse
{
    partial void OnOk(ref bool suppressDefault, ref Widget? result)
    {
        // Call the default deserialization, then patch the result.
        result = DefaultOk();
        if (result != null)
            result.SomeField ??= DeriveValueFromContext();
        suppressDefault = true;   // skip the default path
    }
}

All other ApiResponse classes are unaffected — they continue to use the default deserialization path with no overhead. Partial methods with no implementation are elided entirely by the compiler.

Three usage patterns are supported:

suppressDefault result on entry Effect
false (default) null (default) OnOk is a no-op; DefaultOk() runs as before
true set by OnOk DefaultOk() is skipped entirely; custom result is returned
true set to DefaultOk() then mutated Default deserialization runs inside OnOk, result is patched, DefaultOk() is not called a second time

The DefaultXxx method is private so it is only callable from within the same nested class (including OnXxx implementations in partial classes in the same assembly), preventing misuse from external code.

Affected template

modules/openapi-generator/src/main/resources/csharp/libraries/generichost/api.mustache

The change replaces the single-expression response body with the three-part pattern for every HTTP status code block that has an associated dataType.

PR checklist

  • Read the contribution guidelines.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    (For Windows users, please run the script in WSL)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

Summary by cubic

Adds per-response partial hooks for deserialization in the C# generichost generator so you can customize a single endpoint without overriding AsModel. The previous AsModel logic now lives in DefaultXxx() and AsModel is marked as deprecated.

  • New Features

    • Split generated response methods (e.g., Ok(), status-specific) into a wrapper that calls DefaultXxx().
    • Add partial void OnXxx(ref bool suppressDefault, ref T result) to intercept or replace deserialization per response class.
    • Default behavior is unchanged if the hook isn’t implemented; partials compile away with no overhead.
    • Updated api.mustache; AsModel.mustache now shows a deprecation notice. Samples regenerated to the new pattern.
  • Migration

    • To customize one endpoint, add a partial implementation of OnXxx in the specific generated ApiResponse class.
    • Optionally call DefaultXxx(), modify the result, then set suppressDefault = true. Avoid overriding AsModel.mustache going forward.

Written for commit 0ef1cc5. Summary will update on new commits.

Review in cubic

@devhl-labs

Copy link
Copy Markdown
Contributor Author

The failure is an unrelated java issue.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

30 issues found across 156 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="samples/client/petstore/csharp/generichost/net10/Petstore/src/Org.OpenAPITools/Api/AnotherFakeApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net10/Petstore/src/Org.OpenAPITools/Api/AnotherFakeApi.cs:389">
P2: Setting result (or patching it) inside OnOk without also setting suppressDefault=true is silently discarded because the generated Ok() overwrites result with DefaultOk() right after the hook returns. That makes the documented 'mutate result while letting DefaultOk() run' pattern a footgun - the only way to keep a patch is to call the private DefaultOk() yourself and set suppressDefault, which contradicts the documented workflow. Either update the template/docs so the non-suppressed path preserves an OnOk-provided result (e.g., only default when result is still null), or clarify the docs that patches must combine suppressDefault=true with a manual DefaultOk() call.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net10/AnyOf/src/Org.OpenAPITools/Api/DefaultApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net10/AnyOf/src/Org.OpenAPITools/Api/DefaultApi.cs:318">
P2: When a hook patches `result` without setting `suppressDefault = true`, the generated `if (!suppressDefault) result = DefaultOk();` silently overwrites `result`, so the patch is lost and `Ok()` returns unpatched data. The PR describes patching 'while letting DefaultOk() run' as supported, but the only way to keep a patched result is to call `DefaultOk()` in the hook and set `suppressDefault = true`. Consider documenting this clearly (or adding a guard) so hooks that patch without suppressing don't silently discard their changes.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Api/FakeClassnameTags123Api.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Api/FakeClassnameTags123Api.cs:403">
P2: OnOk runs before the default deserialization, so an implementation that only mutates `result` (intending to patch the deserialized default, as the PR's 'mutate result while letting DefaultOk() run' description suggests) has those mutations silently discarded by the following `if (!suppressDefault) result = DefaultOk();`. The only way to combine default+patch is to manually call the private DefaultOk() inside the hook and set suppressDefault=true, duplicating deserialization. Consider deserializing first and then invoking the hook over the already-populated result so additive patching is the natural path, or make the required suppressDefault contract explicit.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net8/AllOf/src/Org.OpenAPITools/Api/DefaultApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net8/AllOf/src/Org.OpenAPITools/Api/DefaultApi.cs:345">
P2: The documented 'patch result while letting DefaultOk() run' usage does not work with this generated logic: when the OnOk hook assigns result (e.g. result = DefaultOk(); ...) but leaves suppressDefault false, the outer method re-invokes DefaultOk() and silently discards the patched value (also deserializing twice). In practice only the hard-suppress path (suppressDefault=true plus the hook calling DefaultOk() itself) retains a custom result, which contradicts the 'soft override' example in the PR / template docs. Consider adjusting the docs so the only supported customization is setting suppressDefault=true (and having the hook call DefaultOk() when it needs default deserialization), or gate the default call on whether the hook actually set result, so library owners don't hit a silent lost patch.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net10/UseDateTimeForDate/src/Org.OpenAPITools/Api/DefaultApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net10/UseDateTimeForDate/src/Org.OpenAPITools/Api/DefaultApi.cs:319">
P2: The OnOk patch-then-default usage documented in the PR cannot work: when suppressDefault is left false, 'result = DefaultOk();' overwrites any value OnOk assigned to result (and runs DefaultOk() twice), so a caller who patched 'result' silently loses the patch. Consider descending only when OnOk did not already produce a result, e.g. 'if (!suppressDefault && result == null) result = DefaultOk();', so both documented modes behave as described.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net9/SourceGeneration/src/Org.OpenAPITools/Api/AnotherFakeApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net9/SourceGeneration/src/Org.OpenAPITools/Api/AnotherFakeApi.cs:391">
P2: The PR's second documented usage pattern can't work as described: an OnOk that sets result = DefaultOk(), patches, and leaves suppressDefault false gets its patch overwritten (and body deserialized twice) because the caller re-runs DefaultOk() whenever the hook didn't suppress the default. Only the pattern that calls DefaultOk() and then sets suppressDefault = true actually preserves the patch; either correct the usage docs so library owners set suppressDefault = true after patching, or change the template so the default path only fills result when the hook left it null.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net10/FormModels/src/Org.OpenAPITools/Api/FakeClassnameTags123Api.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net10/FormModels/src/Org.OpenAPITools/Api/FakeClassnameTags123Api.cs:402">
P2: The `if (!suppressDefault) result = DefaultOk();` overwrites any value the OnOk hook wrote to `result`, so the PR's documented 'patch result while letting DefaultOk() run' mode cannot work: unless the hook also sets `suppressDefault = true`, its patch is silently discarded and the model is deserialized a second time. Consider applying the default only when the hook leaves `result` unset (or documenting that patching always requires suppressDefault=true) so the hook's patch isn't silently lost.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net9/UseDateTimeForDate/src/Org.OpenAPITools/Api/DefaultApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net9/UseDateTimeForDate/src/Org.OpenAPITools/Api/DefaultApi.cs:319">
P2: When suppressDefault stays false (the default), any value the OnOk partial assigns to result is unconditionally overwritten by result = DefaultOk(), so the documented "patch result while letting DefaultOk() run" pattern silently discards the user's patch unless they also set suppressDefault = true (which then skips the default). If the intent is to allow patching the default deserialization, only fall back to DefaultOk() when the hook didn't initialize result.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net10/AllOf/src/Org.OpenAPITools/Api/DefaultApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net10/AllOf/src/Org.OpenAPITools/Api/DefaultApi.cs:344">
P2: The OnXxx hook's `result` output is only honored when suppressDefault=true; if a user assigns/patch result while leaving suppressDefault false (the PR's documented 'mutate while letting DefaultOk() run' alternative), the guard `if (!suppressDefault) result = DefaultOk();` overwrites it and the patch is silently lost. Clarify the contract so the two usage paths aren't self-contradictory (e.g., always skip DefaultOk when the hook sets suppressDefault OR always honor a non-null hook result), and fix the PR description/test scenario wording to match actual behavior.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net9/NullReferenceTypes/src/Org.OpenAPITools/Api/StoreApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net9/NullReferenceTypes/src/Org.OpenAPITools/Api/StoreApi.cs:756">
P2: The OnOk hook is invoked unconditionally, before the IsOk guard that lives inside DefaultOk(). Previously the whole method was guarded, so a non-200 response returned null without running any user code; now an implemented OnOk fires on every response, and in the suppressDefault=true (custom result) path a hook that deserializes RawContent or accesses model properties will run against 4xx/5xx bodies too. A throw there is caught by TryOk() and logged as a deserialization error, adding log noise/behavior where none existed before. Consider running the hook only when IsOk, or gating the override path on the matching status.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net10/OneOf/src/Org.OpenAPITools/Api/DefaultApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net10/OneOf/src/Org.OpenAPITools/Api/DefaultApi.cs:318">
P2: The documented 'patch while letting DefaultOk() run' usage is a silent no-op: if OnOk sets result and leaves suppressDefault false, the `result = DefaultOk()` line re-runs deserialization and overwrites OnOk's patched value, so the mutation is discarded. Only the suppressDefault=true path (calling DefaultOk() inside OnOk) actually preserves a patch; the PR description lists the other path as supported. Recommend restructuring so OnOk's result isn't clobbered when suppressDefault is false (e.g., only assign result from DefaultOk() when result is still null), or at minimum document that suppressDefault must be true, since a natural OnOk implementation following the docs silently loses its changes.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net8/UseDateTimeForDate/src/Org.OpenAPITools/Api/DefaultApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net8/UseDateTimeForDate/src/Org.OpenAPITools/Api/DefaultApi.cs:317">
P2: OnOk now fires on every call even when the response is not 2xx (IsOk is only checked inside DefaultOk), so a library owner's override runs with side effects on error responses too, where the pre-change method returned null immediately. Consider guarding the hook invocation or documenting/checking IsOk inside OnOk so the success-only hook isn't executed for failure statuses.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net4.7/FormModels/src/Org.OpenAPITools/Api/AnotherFakeApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net4.7/FormModels/src/Org.OpenAPITools/Api/AnotherFakeApi.cs:387">
P2: The OnOk(ref result, suppressDefault) hook makes it impossible to patch the default-deserialized value without either double-deserializing or manually calling DefaultOk() and suppressing. If an implementer sets `result` in OnOk but leaves suppressDefault false (as the PR's documented 'patch after DefaultOk' example implies), Ok() overwrites their result with a fresh DefaultOk() call and the customization is silently discarded plus RawContent is deserialized twice. Consider documenting that suppressDefault must be set after patching, or restructure so the hook can post-process the default result in a single deserialization (e.g. have Ok() run DefaultOk() first and pass its value into OnOk).</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Api/FakeClassnameTags123Api.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Api/FakeClassnameTags123Api.cs:403">
P2: The OnOk hook is invoked before DefaultOk(), so `result` is always null when the hook runs, and any value assigned to it in OnOk is silently overwritten by the subsequent `result = DefaultOk()` unless the user also sets suppressDefault = true. The documented "patch result while letting DefaultOk() run" workflow cannot work, and the `ref result` parameter is misleading because it can never receive the default-deserialized value (patching forces re-running DefaultOk() inside the hook). Consider running DefaultOk() first into result and then invoking OnOk(ref suppressDefault, ref result) so the hook can genuinely patch the deserialized value, or drop the ref-result patching expectation from the docs/signature.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net8/AnyOfNoCompare/src/Org.OpenAPITools/Api/DefaultApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net8/AnyOfNoCompare/src/Org.OpenAPITools/Api/DefaultApi.cs:319">
P2: The patch-path is a silent trap: when an OnOk hook sets `result = DefaultOk(); ...patch...` but leaves `suppressDefault` false (exactly what the PR's "how to use" section describes: 'mutate result while letting DefaultOk() run'), the caller then executes `result = DefaultOk()` again, re-deserializing RawContent and discarding the patched value. The patch only survives if suppressDefault=true, which the PR text does not state for this scenario. Recommend documenting on the DefaultOk/OnOk members that setting result requires suppressDefault=true, and note the double-deserialization cost of calling DefaultOk() within OnOk.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net8/AnyOf/src/Org.OpenAPITools/Api/DefaultApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net8/AnyOf/src/Org.OpenAPITools/Api/DefaultApi.cs:319">
P2: When OnOk assigns a non-null result but leaves suppressDefault=false, the outer flow immediately overwrites it with result = DefaultOk(), so the hook's patched value is silently discarded. As written, the PR's documented "mutate result while letting DefaultOk() run" pattern cannot work (and calling DefaultOk() manually to seed result causes a redundant second deserialization) — the only working patch path requires setting suppressDefault=true and calling DefaultOk() manually. Consider documenting that constraint or restructuring so a hook-produced result is not unconditionally overwritten.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/latest/UseDateTimeOffset/src/Org.OpenAPITools/Api/FakeClassnameTags123Api.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/latest/UseDateTimeOffset/src/Org.OpenAPITools/Api/FakeClassnameTags123Api.cs:404">
P2: The OnOk contract silently discards any result a hook assigns unless it also sets suppressDefault=true, because `if (!suppressDefault) result = DefaultOk();` unconditionally overwrites the hook's value. A library owner following the PR's 'mutate result while letting DefaultOk() run' example (assigning result = DefaultOk() and patching, without suppressDefault) will have their patch overwritten and DefaultOk() invoked twice. Recommend documenting explicitly (in the Ok()/OnOk doc comment and PR example) that keeping any custom result requires suppressDefault=true, or restructuring the hook signature to make the default-vs-custom decision unambiguous.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Api/FakeClassnameTags123Api.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Api/FakeClassnameTags123Api.cs:402">
P2: The documented "mutate result while letting DefaultOk() run" pattern cannot work: the outer method unconditionally does `result = DefaultOk()` whenever `suppressDefault` is false, so any patch the OnOk hook applies to `result` is silently overwritten and never returned. In the implemented contract the only way a hook can keep a customization is to also set `suppressDefault = true` (option 1) and call DefaultOk() itself — the second usage path described in the PR is non-functional. Either drop that usage case from the docs or restructure so the default is only applied when the hook left `result` unset.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Api/AnotherFakeApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Api/AnotherFakeApi.cs:390">
P2: The OnOk patch path silently double-deserializes: if a user calls DefaultOk() and patches result inside OnOk without setting suppressDefault=true, the outer `if (!suppressDefault) result = DefaultOk();` re-runs DefaultOk(), overwriting the patched result with a fresh deserialize of RawContent. The PR's recommended 'set result to DefaultOk() first, then patching' wording is misleading, since the only working form additionally requires suppressDefault=true. Recommend clarifying the hook contract (e.g. documenting that patch requires suppressDefault=true, or passing the already-deserialized result into the hook) to avoid silently lost patches.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net8/SourceGeneration/src/Org.OpenAPITools/Api/AnotherFakeApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net8/SourceGeneration/src/Org.OpenAPITools/Api/AnotherFakeApi.cs:392">
P2: The PR documents a usage where OnOk patches result while letting the default path run ("setting result to DefaultOk() first, then patching"), but the generated Ok() unconditionally does `result = DefaultOk()` when suppressDefault stays false, so any patch applied to result inside OnOk is silently discarded. Only the suppressDefault=true + manual DefaultOk() call path works (as test case 3 confirms); reword the usage docs or guard the reassignment (e.g. only assign when suppressDefault is false AND result is unchanged) so the documented patch-without-suppress scenario isn't a footgun.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/standard2.0/Petstore/src/Org.OpenAPITools/Api/AnotherFakeApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/standard2.0/Petstore/src/Org.OpenAPITools/Api/AnotherFakeApi.cs:386">
P2: The OnOk hook is invoked before any status check, so it fires on non-success responses too, whereas the old path and DefaultOk only deserialize when IsOk. A hook written purely for the success case will be called on error responses and, if it suppresses the default, can yield a non-null result for a failing status. Consider gating the OnOk call on IsOk (or documenting that hooks must guard with IsOk/DefaultOk's status check) so error responses keep returning null.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Api/AnotherFakeApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Api/AnotherFakeApi.cs:404">
P2: The OnOk(ref ..., ref result) hook is a footgun: result assigned inside OnOk is silently discarded by DefaultOk() unless the implementer also sets suppressDefault=true, with no inline hint in the generated code. The only cue is the 'NOTICE' comment inside DefaultOk() (which an OnOk implementer may never open). Recommend documenting the suppressDefault requirement directly above the partial method declaration (e.g. a /// remarks tag stating that result is only honored when suppressDefault=true), so the contract is discoverable at the hook site.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/latest/AnnotatedEnum/src/Org.OpenAPITools/Api/DefaultApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/latest/AnnotatedEnum/src/Org.OpenAPITools/Api/DefaultApi.cs:343">
P2: The Ok() flow re-runs DefaultOk() after the OnOk hook whenever suppressDefault is false. If a hook follows the documented "set result = DefaultOk() first, then patch, while letting DefaultOk run" guidance, the response body is deserialized twice and the second DefaultOk() overwrites the patched result, silently discarding the customization. The guide should instruct hooks to set suppressDefault = true after calling DefaultOk(), or the hook should be the single deserialization point.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net4.8/AllOf/src/Org.OpenAPITools/Api/DefaultApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net4.8/AllOf/src/Org.OpenAPITools/Api/DefaultApi.cs:342">
P2: The generated flow calls `OnOk(...)` and then unconditionally runs `result = DefaultOk()`, which overwrites any value `OnOk` assigned to `result` whenever `suppressDefault` remains false. This makes the documented "mutate result while letting DefaultOk() run, by setting result to DefaultOk() first, then patching" workflow impossible: the patch is silently discarded, so the only usable path is calling `DefaultOk()` inside `OnOk` and setting `suppressDefault = true` (equivalent to the first bullet). Consider restructuring so `DefaultOk()` runs first and the hook is invoked afterward, or correcting the documentation/example to reflect that patching requires `suppressDefault = true`, to avoid users following the docs and losing their patched result.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net9/NullReferenceTypes/src/Org.OpenAPITools/Api/FakeClassnameTags123Api.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net9/NullReferenceTypes/src/Org.OpenAPITools/Api/FakeClassnameTags123Api.cs:405">
P2: The generated Ok() discards any `result` value that an OnOk hook assigns unless the hook also sets suppressDefault=true: `result = DefaultOk()` unconditionally overwrites it afterwards. This contradicts the PR's documented patch path ('mutate result while letting DefaultOk() run'), which as written would silently lose the patch and deserialize twice; consider documenting that patching requires suppressDefault=true (calling DefaultOk() inside the hook), or making the generated code prefer a non-null hook-initialized result.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Api/FakeClassnameTags123Api.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Api/FakeClassnameTags123Api.cs:403">
P2: The documented 'patch while letting DefaultOk() run' usage is not achievable with this code: when suppressDefault stays false, the main method's `result = DefaultOk()` unconditionally overwrites anything OnOk set on `result`, so any patch applied after calling `result = DefaultOk()` inside the hook is silently discarded. Patching only works when OnOk sets suppressDefault=true and calls DefaultOk() itself — consider either updating the docs to reflect that single working path or changing the flow so an already-assigned `result` from OnOk isn't clobbered.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/latest/NullTypes/src/Org.OpenAPITools/Api/DefaultApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/latest/NullTypes/src/Org.OpenAPITools/Api/DefaultApi.cs:338">
P2: The hook silently discards a caller's patched `result` unless they also set `suppressDefault = true`, which makes the documented "patch while letting DefaultOk run" path lose its patch and deserialize twice. Consider only falling back to the default when the hook left `result` null (e.g. `if (!suppressDefault && result is null) result = DefaultOk();`) so a patched value is preserved, and be aware the current overload contract requires the suppress flag even for patching.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net4.8/OneOf/src/Org.OpenAPITools/Api/DefaultApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net4.8/OneOf/src/Org.OpenAPITools/Api/DefaultApi.cs:316">
P2: The OnOk hook runs before DefaultOk in Ok(), so the PR-documented 'patch while letting DefaultOk run' usage double-deserializes and silently discards the patch: DefaultOk() is called inside OnOk and again in `if (!suppressDefault) result = DefaultOk();`. OnOk has no way to patch the value DefaultOk computes unless it sets suppressDefault=true and re-implements the deserialization call itself; the usage examples and test scenario 3 should be reconciled (either document that patching requires suppressDefault=true + manual DefaultOk, or have the hook run after the default deserialization so it can patch it).</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net4.8/FormModels/src/Org.OpenAPITools/Api/AnotherFakeApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net4.8/FormModels/src/Org.OpenAPITools/Api/AnotherFakeApi.cs:387">
P2: The "mutate result while letting DefaultOk() run" usage advertised in the PR does not work: any value written into `result` inside `OnOk` is unconditionally overwritten by `result = DefaultOk()` when `suppressDefault` stays false, so a library owner's patch is silently discarded (and DefaultOk() deserializes a second time). To keep a patch, the owner must call `DefaultOk()` inside the hook and set `suppressDefault = true`, and the generation should be restructured (or documented clearly) so `result` set in `OnOk` survives when the default path also runs.</violation>
</file>

<file name="samples/client/petstore/csharp/generichost/net9/Petstore/src/Org.OpenAPITools/Api/AnotherFakeApi.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net9/Petstore/src/Org.OpenAPITools/Api/AnotherFakeApi.cs:388">
P2: The `ref result` on OnOk is silently discarded whenever suppressDefault stays false: the very next line overwrites it with DefaultOk(). A library owner who patches `result` in OnOk without also flipping suppressDefault and calling DefaultOk() themselves gets the plain default with no error, which is an easy-to-misuse footgun in the primary customization hook's contract. Consider documenting this clearly or restructuring so the hook's result isn't silently dropped.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

@wing328 wing328 added Client: C-Sharp Enhancement: Code Cleanup General refactoring, removal of deprecated things, commenting, etc. labels Aug 9, 2026
@wing328 wing328 added this to the 7.25.0 milestone Aug 9, 2026
@wing328

wing328 commented Aug 9, 2026

Copy link
Copy Markdown
Member

@wing328
wing328 merged commit 32e5f0b into OpenAPITools:master Aug 9, 2026
76 of 77 checks passed
@devhl-labs
devhl-labs deleted the devhl/refactor-AsModel-template branch August 9, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client: C-Sharp Enhancement: Code Cleanup General refactoring, removal of deprecated things, commenting, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants