[zeynepstariongroup]

Prerequisites

• I have written a descriptive pull-request title
• I have verified that there are no overlapping pull-requests open
• I have verified that I am following the SysML2.NET code style guidelines
• I have provided test coverage for my change (where applicable)

Description

Fix #404 the OpenAPI description is used to create the 35 API functionalities. After the main development is done, I included additional corrections to be compatible with the mycelium Fabric project naming convention.

[antoineatstariongroup]

The generatedCode should be put on the class, not on the method

[antoineatstariongroup]

Move this into a specific Page constructor

[antoineatstariongroup]

Instanciate a JsonWriterOption, as static property of the class, to not have the need to allocate new instance on each call

[antoineatstariongroup]

Can't we directly provide the generic on the DeSerializeAsync, to not have to do casting later?

[antoineatstariongroup]

Can't we directly provide the generic on the DeSerializeAsync, to not have to do casting later?

[antoineatstariongroup]

same comment as the other exception

[antoineatstariongroup]

Never update PackageReleaseNotes, it's only at release time that we are updating it

[samatstariongroup]

The generated client was compared operation by operation against Resources/ptc-25-02-30.json: all 35 operations are present once, with matching HTTP method, path template, query parameter names and success response types. The solution builds without errors and all tests pass locally. The SonarQube quality gate passes with no open issues. The points below concern behaviour against the OpenAPI, the generator templates, and the scope of the change.

[samatstariongroup]

Page tokens are parsed into a fixed format

SySML2.NET.REST/PsmApiClient.cs:153-211 (QueryLinkedPage / ParsePage), called from CreatePagedResult (:513-521) on every paged response.

The OpenAPI declares page[after] and page[before] as {"type": "string"} with no format. ParsePage requires the token to be epoch|guid and throws FormatException otherwise. Because CreatePagedResult runs after a successful response, a server issuing tokens in another format causes all 15 paged operations to throw, and the records already read are discarded. QueryParameters.PageAfter / PageBefore are typed Page?, so a token returned by such a server cannot be passed back either.

Suggestion: carry the page token as an opaque string. If Page is kept, an unparseable token should not fail the call.

[samatstariongroup]

Directory.Packages.props changes beyond the new packages

Ignoring whitespace, the diff is three lines: Microsoft.Extensions.Http 8.0.1 and RichardSzalay.MockHttp 7.0.0 are added, and MessagePack goes from 3.1.8 to 3.1.12. The remaining +44/-48 lines come from reformatting (4-space to 2-space indentation, blank lines between groups removed, trailing newline removed).

Suggestion: keep the original formatting, and move the MessagePack update to a separate PR since it is not related to #404.

[samatstariongroup]

Deletion of SysML2.NET.CodeGenerator/Resources/SysML.ecore

No file in the solution references it, so deleting it breaks nothing. It is not related to #404.

Suggestion: move it to a separate commit or PR.

[samatstariongroup]

409 response with a body other than DataIdentity[]

SySML2.NET.REST/PsmApiClient.cs:363-375 (ThrowOnConflictAsync) deserializes every JSON 409 as DataIdentity[]. If the body is an Error or malformed, the deserializer exception propagates and the status code is lost. EnsureSuccessAsync (:335-342) catches the same exception types and falls back to PsmApiException.

Suggestion: apply the same catch … when filter and fall through to EnsureSuccessAsync.

[samatstariongroup]

Required sourceCommitId can be omitted

merge declares sourceCommitId with "required": true (the PIM mergeIntoBranch::commitsToMerge is [1..*]). In MergeAsync (SySML2.NET.REST/AutoGenPsmApiClient/DiffMergeApi.cs:109), a null or empty sequence is formatted as null by QueryStringBuilder.FormatSequence (SySML2.NET.REST/QueryStringBuilder.cs:98-102). The parameter is left out and the request is sent anyway.

Suggestion: throw ArgumentException (or ArgumentNullException) for required array parameters that are null or empty.

[samatstariongroup]

GetDatatypesAsync cannot reach the next page

getDatatypes declares page[after], page[before] and page[size], and its 200 response is {"type": "object"}. GetDatatypesAsync (MetaApi.cs:51) accepts QueryParameters but returns a JsonDocument, so the Link header is never read and the caller has no way to get the next page.

[samatstariongroup]

Operation-specific assumptions in the templates

All of these produce correct output for the current OpenAPI, but they are not derived from it:

• psm-apiclient-openapi-template.hbs:59-61: any 409 response emits ThrowOnConflictAsync (DataIdentity[] → MergeConflictException). The response schema is not consulted.
• psm-apiclient-openapi-template.hbs:43,46 and psm-apiclient-signature-openapi-partial.hbs:29,32: any parameter whose name starts with page is dropped from the signature, and QueryParameters is added only when page[size] is declared. An operation declaring only page[after] would lose it.
• psm-apiclient-signature-openapi-partial.hbs:12 and psm-apiclient-openapi-template.hbs:68: one return type and one return are emitted per 2xx JSON response. An operation with two such responses would generate code that does not compile.
• Every array query parameter is joined with commas, whatever its style/explode. This is correct for sourceCommitId, the only array parameter today.

[samatstariongroup]

new HttpMethod("…") in generated code

psm-apiclient-openapi-template.hbs:54,56 emit new HttpMethod("{{HttpMethod}}"), which appears 35 times in the generated partials. The OpenAPI uses only GET, POST, PUT and DELETE, and all four exist as static members (HttpMethod.Get, .Post, .Put, .Delete). Emitting the static members makes an invalid method name a compile error and avoids an allocation per call. Behaviour does not change, since HttpMethod equality is by method name.

[samatstariongroup]

Mixed using var / await using var for response streams

SySML2.NET.REST/PsmApiClient.cs uses using var stream at lines 333, 370, 399, 421 and 448, and await using var stream at lines 472 and 493. Stream implements IAsyncDisposable on netstandard2.1.

Suggestion: use await using consistently.

[samatstariongroup]

Query string assertions in PsmApiClientTestFixture

The client tests match query parameters with .WithQueryString(name, value) (SySML2.NET.REST.Tests/PsmApiClientTestFixture.cs:101-102, 191, 216, 244, 275-276, 333-334). This passes when the named parameter is present, even if other parameters are added or a required one is missing (see point 5). .WithExactQueryString(…) fixes the complete query string: names, order, encoding, and no extra parameters.

[samatstariongroup]

Follow-up on d0bcf63. Two points about conformance to the Systems Modeling API and Services specification and the OpenAPI document; details inline.

[samatstariongroup]

An unparseable page token ends pagination without an error

With an unparseable token, QueryLinkedPage logs a warning and returns null, so PagedResult.Next (or Previous) is null. The not-a-page case in PsmApiClientTestFixture.cs:129-134 asserts this behaviour.

Systems Modeling API and Services, Clause 8.1.3 (Pagination) describes the strategy as cursor-based. The page[after] / page[before] values appear only in examples (MTYxODg2MTQ5NjYzMnwy…). The clause tells the client to "continue … until the Link header in the response does not include the URL for the next page (rel value as next)". A null Next is therefore the spec's end-of-results signal. Here it is also returned when a next link is present but its token does not match the epoch|guid encoding, so a caller stops early and the remaining records are not retrieved. The OpenAPI declares both parameters as {"type": "string"} with no format.

Related: the XML doc of Page.TryParse (SysML2.NET/PSM/Page.cs:35) states that Clause 8.1.3 defines the token as the unpadded Base64 encoding of an epoch and an identifier. The clause shows such tokens in its examples but does not define the format.

Suggestion: keep the token exactly as the server returned it (a string on Page, or string next/previous tokens on PagedResult and QueryParameters), and send it back unchanged. Epoch/Identifier can stay as an optional decoded view. If a fixed format is kept, an unparseable token should surface as an exception instead of looking like the last page. In both cases the doc comment should refer to Clause 8.1.3 as an example, not a definition.

[samatstariongroup]

The comma delimiter of sourceCommitId is percent-encoded

FormatSequence joins the values with ,, then Add (line 74) applies Uri.EscapeDataString to the whole string. The request therefore carries sourceCommitId=<id1>%2C<id2>, which PsmApiClientTestFixture.cs:368 asserts.

ptc-25-02-30.json declares the parameter with "style": "form", "explode": false. For an array, the OpenAPI 3.1 serialization of that style is sourceCommitId=<id1>,<id2>: the comma is the delimiter and is not encoded, while the individual values are. A server that splits the raw query value on , before decoding receives a single value. Servers that decode first are not affected.

Suggestion: escape each value, join the escaped values with a literal ,, and update the expected query string in the test.

[antoineatstariongroup]

group exceptions inside a common folder "Exceptions" and align namespace into : SysML2.NET.REST.Exceptions

[samatstariongroup]

Thanks for this, the client is in good shape: build is clean and all 1,466 tests pass. One change is needed before merge.

A required ID or request body can be null or empty, and the request still goes out

SysML2.NET.CodeGenerator/Templates/OpenAPI/psm-apiclient-openapi-template.hbs only guards required parameters when they are arrays. Required string parameters and required request bodies are sent unchecked.

Failure: GetDatatypeByIdAsync(null) or GetDatatypeByIdAsync("") sends a GET to meta/datatypes/ (because Escape(null) returns ""). That is the getDatatypes collection endpoint. A server that accepts the trailing slash returns the whole datatype list, and the caller receives it as if it were the single datatype, with no error. A required body passed as null (for example PostBranchByProjectAsync(projectId, null), where the OpenAPI declares requestBody.required: true) is posted without a body, and the caller only finds out from a server 4xx.

Affected operations (from Resources/ptc-25-02-30.json):

• required string parameter: getDatatypeById (datatypeId, format: uri). Every other required ID is a Guid, so it cannot be null.
• required request body: postBranchByProject, postTagByProject, postCommitByProject, postQueryByProject, putQueryByProjectAndId, getQueryResultsByProjectIdQuery, getQueryResultsByProjectIdQueryPost

VerifyPostCommitByProjectAsync currently asserts that a null body is sent and comes back as a 400. If that was deliberate, let's discuss. Otherwise the client should reject a value the OpenAPI marks as required before any request goes out.

Suggested fix

After the change, regenerate SySML2.NET.REST/AutoGenPsmApiClient/ and SysML2.NET.CodeGenerator.Tests/Expected/OpenAPI/REST/AutoGenPsmApiClient/.

1. SysML2.NET.CodeGenerator/Templates/OpenAPI/psm-apiclient-openapi-template.hbs: replace the guard block (lines 54–61). The Enum/uuid exclusions match the signature partial, which turns those parameters into an enum or a Guid.

            {{#each Parameters}}
            {{#if Required}}
            {{#if (String.Equal (OpenApi.Text Schema.Type) 'Array')}}
            ThrowIfNullOrEmpty({{Name}}, nameof({{Name}}));

            {{else if (String.Equal (OpenApi.Text Schema.Type) 'String')}}
            {{#unless Schema.Enum}}
            {{#unless (String.Equal Schema.Format 'uuid')}}
            ThrowIfNullOrWhiteSpace({{Name}}, nameof({{Name}}));

            {{/unless}}
            {{/unless}}
            {{/if}}
            {{/if}}
            {{/each}}
            {{#if Operation.RequestBody.Required}}
            ThrowIfNull(requestBody, nameof(requestBody));

            {{/if}}

2. SysML2.NET.CodeGenerator/Templates/OpenAPI/Partials/psm-apiclient-documentation-openapi-partial.hbs: document the new exceptions. Add this after the array <exception> block (before the {{/if}} closing the Array test, line 43):

{{else if (String.Equal (OpenApi.Text Schema.Type) 'String')}}
{{#unless Schema.Enum}}
{{#unless (String.Equal Schema.Format 'uuid')}}
/// <exception cref="ArgumentNullException">
/// Thrown when <paramref name="{{Name}}"/> is <c>null</c>
/// </exception>
/// <exception cref="ArgumentException">
/// Thrown when <paramref name="{{Name}}"/> is empty or whitespace
/// </exception>
{{/unless}}
{{/unless}}

and add this after the {{/each}} on line 45:

{{#if Operation.RequestBody.Required}}
/// <exception cref="ArgumentNullException">
/// Thrown when <paramref name="requestBody"/> is <c>null</c>
/// </exception>
{{/if}}

3. SySML2.NET.REST/PsmApiClient.cs: add these helpers next to ThrowIfNullOrEmpty. ArgumentNullException.ThrowIfNull is not available on netstandard2.1.

        /// <summary>
        /// Throws when a required text parameter is <c>null</c>, empty or whitespace
        /// </summary>
        /// <param name="value">
        /// The value of the parameter
        /// </param>
        /// <param name="parameterName">
        /// The name of the parameter
        /// </param>
        /// <exception cref="ArgumentNullException">
        /// Thrown when <paramref name="value"/> is <c>null</c>
        /// </exception>
        /// <exception cref="ArgumentException">
        /// Thrown when <paramref name="value"/> is empty or whitespace
        /// </exception>
        private static void ThrowIfNullOrWhiteSpace(string value, string parameterName)
        {
            if (value is null)
            {
                throw new ArgumentNullException(parameterName);
            }

            if (string.IsNullOrWhiteSpace(value))
            {
                throw new ArgumentException("A non-whitespace value is required", parameterName);
            }
        }

        /// <summary>
        /// Throws when a required request body is <c>null</c>
        /// </summary>
        /// <param name="value">
        /// The request body
        /// </param>
        /// <param name="parameterName">
        /// The name of the parameter
        /// </param>
        /// <exception cref="ArgumentNullException">
        /// Thrown when <paramref name="value"/> is <c>null</c>
        /// </exception>
        private static void ThrowIfNull(object value, string parameterName)
        {
            if (value is null)
            {
                throw new ArgumentNullException(parameterName);
            }
        }

4. SySML2.NET.REST.Tests/PsmApiClientTestFixture.cs

In VerifyGetDatatypeByIdAsync:

            using (Assert.EnterMultipleScope())
            {
                await Assert.ThatAsync(() => this.client.GetDatatypeByIdAsync(null), Throws.ArgumentNullException);
                await Assert.ThatAsync(() => this.client.GetDatatypeByIdAsync(" "), Throws.ArgumentException);
            }

In VerifyPostCommitByProjectAsync, replace the null-body 400 scenario. The 400 case keeps a body, so it still checks that branchId is left out of the query string:

            this.mockHttp.Expect(HttpMethod.Post, commitsUri)
                .With(request => string.IsNullOrWhiteSpace(request.RequestUri.Query))
                .Respond(HttpStatusCode.BadRequest, JsonMediaType, ErrorPayload);

            await Assert.ThatAsync(() => this.client.PostCommitByProjectAsync(ProjectId, commitRequest),
                Throws.TypeOf<PsmApiException>().With.Property(nameof(PsmApiException.StatusCode)).EqualTo(HttpStatusCode.BadRequest));

            await Assert.ThatAsync(() => this.client.PostCommitByProjectAsync(ProjectId, null), Throws.ArgumentNullException);

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
98.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[samatstariongroup]

fantastic job!