CP-2062 - Generate credentials and dashboards after changes #11
Conversation
| @@ -55,7 +56,7 @@ | |||
| import java.util.Set; | |||
| import java.util.function.Consumer; | |||
|
|
|||
| @jakarta.annotation.Generated(value = "com.thousandeyes.api.codegen.ThousandeyesJavaGenerator", date = "2024-04-22T10:43:50.463323+01:00[Europe/London]") | |||
There was a problem hiding this comment.
ffs do we really need this timestamp ? ends up adding unnecessary changes to PR
There was a problem hiding this comment.
We can add hideGenerationTimestamp: true to common.yaml... although, since this is a custom template we can just remove the timestamps from the template entirely 🤷
There was a problem hiding this comment.
I think it adds value once this is auto generated, then we know what has changed, but rn it is just adding more file changes to PRs 😢
There was a problem hiding this comment.
I think it adds value once this is auto generated
I thought the same initially, but I'm not even so sure any more... in the post-release action I assume we're going to regenerate all the SDKs using the latest OAS, and so presumably we'll have the same problem (albeit with more automation)
Or do you mean we'll be able to know which models need to be regenerated, and only those will be generated/updated?
| @@ -82,7 +82,7 @@ public class Example { | |||
| ### HTTP request headers | |||
|
|
|||
| - **Content-Type**: application/json | |||
| - **Accept**: application/hal+json, application/problem+json | |||
| - **Accept**: application/hal+json, application/json, application/problem+json | |||
There was a problem hiding this comment.
Adding application/json to all
| @@ -223,8 +224,8 @@ private ApiRequest.ApiRequestBuilder getTransactionTestsCredentialDetailsRequest | |||
| requestBuilder.queryParams(localVarQueryParams); | |||
| } | |||
|
|
|||
| requestBuilder.header("Accept", List.of("application/hal+json, application/problem+json")); | |||
| requestBuilder.header("User-Agent", List.of("ThousandEyesSDK-Java/7.0.0")); | |||
| requestBuilder.header("Accept", List.of("application/hal+json, application/json, application/problem+json")); | |||
There was a problem hiding this comment.
application/json added to endpoints with request
| @@ -169,7 +170,7 @@ private ApiRequest.ApiRequestBuilder deleteTransactionTestsCredentialRequestBuil | |||
| } | |||
|
|
|||
| requestBuilder.header("Accept", List.of("application/problem+json")); | |||
There was a problem hiding this comment.
but not yet added to endpoints without requests
| */ | ||
|
|
||
| @Test | ||
| public void createTransactionTestsCredentialRequestAndResponseDeserializationTest() |
There was a problem hiding this comment.
Good example with proper example generated
| * | ||
| * @throws JsonProcessingException if the deserialization fails | ||
| */ | ||
| @Disabled |
There was a problem hiding this comment.
No request nor response, disabled test
| */ | ||
|
|
||
| @Test | ||
| public void getTransactionTestsCredentialDetailsRequestAndResponseDeserializationTest() |
There was a problem hiding this comment.
Bad example generation because of the composed schema bug
| throws JsonProcessingException | ||
| { | ||
|
|
||
| String responseBodyJson = """ |
| } ] | ||
| """; | ||
| List<ApiDashboard> mappedResponse = | ||
| mapper.readValue(responseBodyJson, new TypeReference<List<ApiDashboard>>(){}); |
There was a problem hiding this comment.
TypeReference handling for list responses
| "title" : "HTTP Server Widgets", | ||
| "isBuiltIn" : true, | ||
| "widgets" : [ { | ||
| "type" : "Agent Status", |
There was a problem hiding this comment.
widgets are missing attributes bcs of the bug
| { | ||
| String requestBodyJson = """ | ||
| { | ||
| "context" : [ { |
There was a problem hiding this comment.
unless maxItems is defined on OAS, by default, the ExampleGenerator will add 2 examples for every array
| """; | ||
| CredentialRequest mappedRequest = | ||
| mapper.readValue(requestBodyJson, CredentialRequest.class); | ||
| assertNotNull(mappedRequest); |
There was a problem hiding this comment.
is it possible/easy to verify the fields have values?
There was a problem hiding this comment.
possible ? most likely yea, by using reflection, but easy ? I dont think so.
If this was on the model test would be a lot easier, cause the model codegen has all its fields and whether they are required and what not, but here on the operation cogeden we only have the reference :/
Two samples generated from code on PR/9.
Some examples are missing fields, depending on this BUG to be fixed on the generator.