Fix OpenAPI schemas and request header sizing - #107
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb0138bf2e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (String propertyName : object.getProperties().keySet()) { | ||
| object.addRequiredItem(propertyName); |
There was a problem hiding this comment.
When an entity defines an optional DATE or OBJECT field without a default, JsonThing.asFieldJsonObject omits that field because InstanceFields.getFieldValue returns null, but this loop now marks every defined property as required. Collection responses containing such entities therefore fail validation against the generated OpenAPI schema and generated clients assume fields that may be absent; only fields guaranteed to be rendered should be added to required.
Useful? React with 👍 / 👎.
| XML xml = new XML(); | ||
| xml.setWrapped(true); | ||
| arrayObject.setXml(xml); | ||
| collectionObject.setXml(xml); | ||
| collectionObject.addProperties(objectSchemaDefinition.getPlural(), arrayObject); |
There was a problem hiding this comment.
For application/xml collection responses, this object property adds another plural-named layer to the schema: the endpoint emits <items><item>...</item></items>, while the new object schema has an outer collection object containing an items property. Setting wrapped on the outer object does not flatten that property because XML wrapping applies to arrays, and responseContentWith uses this same component for both JSON and XML, so XML client generation and validation no longer match the actual response.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates the Thingifier OpenAPI/Swagger generation and Javalin/Jetty server configuration to better match real API behavior and support larger browser-originated request headers.
Changes:
- Update Swaggerizer to emit OpenAPI schema examples using correctly-typed JSON values (numbers/booleans not serialized as strings).
- Update collection response schemas to describe the
{ "<plural>": [...] }wrapper object returned by the API. - Add configurable Jetty request header size in the Javalin adapter, with regression tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| thingifier/src/test/java/uk/co/compendiumdev/thingifier/swaggerizer/SwaggerizerSchemaExampleTest.java | Adds regression tests for typed examples and collection-wrapper response schemas. |
| thingifier/src/test/java/uk/co/compendiumdev/thingifier/adapter/javalin/JavalinHttpServerTest.java | Adds tests for request header size defaults/configuration and acceptance of large Cookie headers. |
| thingifier/src/main/java/uk/co/compendiumdev/thingifier/swaggerizer/Swaggerizer.java | Adjusts schema generation for collection wrappers, required fields, and example typing. |
| thingifier/src/main/java/uk/co/compendiumdev/thingifier/adapter/javalin/JavalinHttpServer.java | Adds Jetty HttpConfiguration customization for request header sizing via property/env/default. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| XML xml = new XML(); | ||
| xml.setWrapped(true); | ||
| arrayObject.setXml(xml); | ||
| collectionObject.setXml(xml); | ||
| collectionObject.addProperties(objectSchemaDefinition.getPlural(), arrayObject); | ||
| collectionObject.addRequiredItem(objectSchemaDefinition.getPlural()); |
| private static ObjectSchema asRequiredResponseObjectSchema( | ||
| EntityDefinition objectSchemaDefinition) { | ||
| ObjectSchema object = asObjectSchema(objectSchemaDefinition); | ||
| if (object.getProperties() != null) { | ||
| for (String propertyName : object.getProperties().keySet()) { | ||
| object.addRequiredItem(propertyName); | ||
| } | ||
| } | ||
| return object; | ||
| } |
| // add list response for entity plural | ||
| ArraySchema arrayObject = asArrayObjectSchema(objectSchemaDefinition); | ||
| ObjectSchema arrayObject = asArrayObjectSchema(objectSchemaDefinition); | ||
| components.addSchemas(objectSchemaDefinition.getPlural(), arrayObject); |
Summary
{ "items": [...] }, with required fields on returned item objects.Validation
mvn -pl thingifier "-Dtest=Swaggerizer*Test,OpenApi32FinalizerTest,SwaggerUiPageTest" testpassed: 16 tests.mvn install "-DskipTests=true" "-Dcheckstyle.skip=true" "-Dpmd.skip=true"passed and installed all reactor modules to local Maven.Notes
A full
mvn installcurrently fails on pre-existing project-FQN Checkstyle violations outside this change. Running with static checks skipped and tests enabled reachedthingifier-crud-uiintegration tests, where two existing e2e assertions failed (DELETEexpected 200 but received 204, and Swagger UI no longer contains the wordExplore).