[csharp][generichost] Refactor AsModel template - #24650
Merged
wing328 merged 1 commit intoAug 9, 2026
Merged
Conversation
Contributor
Author
|
The failure is an unrelated java issue. |
Contributor
There was a problem hiding this comment.
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
Member
|
https://github.com/OpenAPITools/openapi-generator/actions/runs/31285200676/job/93172621679?pr=24650 already fixed in the master |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[csharp][generichost] Add
OnXxxpartial hook to response deserialization methodsProblem
The only supported customization point for response deserialization in the
generichostlibrary is theAsModel.mustachetemplate. Overriding that template replaces the body of everyOk()method (and its siblings for other HTTP status codes) across every generatedApiResponseclass. 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
AfterXxxpartial hook runs after the full API call and receives theApiResponseobject, but modifications to the deserialized result made there are discarded: the next caller ofOk()triggers a fresh deserialization fromRawContent, 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-OkHTTP status code) into three parts:The
partial void OnXxxmethod follows the same suppress-default pattern already used byAfterXxxandOnErrorXxxin the outer API class. Library owners implement it in a partial class for only the specific response classes they need to customize:All other
ApiResponseclasses 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:
suppressDefaultresulton entryfalse(default)null(default)OnOkis a no-op;DefaultOk()runs as beforetrueOnOkDefaultOk()is skipped entirely; custom result is returnedtrueDefaultOk()then mutatedOnOk, result is patched,DefaultOk()is not called a second timeThe
DefaultXxxmethod isprivateso it is only callable from within the same nested class (includingOnXxximplementations in partial classes in the same assembly), preventing misuse from external code.Affected template
modules/openapi-generator/src/main/resources/csharp/libraries/generichost/api.mustacheThe 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
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.
Summary by cubic
Adds per-response partial hooks for deserialization in the C#
generichostgenerator so you can customize a single endpoint without overridingAsModel. The previousAsModellogic now lives inDefaultXxx()andAsModelis marked as deprecated.New Features
Ok(), status-specific) into a wrapper that callsDefaultXxx().partial void OnXxx(ref bool suppressDefault, ref T result)to intercept or replace deserialization per response class.api.mustache;AsModel.mustachenow shows a deprecation notice. Samples regenerated to the new pattern.Migration
OnXxxin the specific generatedApiResponseclass.DefaultXxx(), modify the result, then setsuppressDefault = true. Avoid overridingAsModel.mustachegoing forward.Written for commit 0ef1cc5. Summary will update on new commits.