readOnly: true not supported

[mirek26]

👋🏽

First of all, thanks for writing this library - I'm looking for an openapi client generator for internal APIs and I tried 3 other options and this was the only tool that produced code that compiles without issues. I hit some random issues (eg example generation panics) but I like the code this produces and it has most of the features I'd like (enums would be the next on my wishlist ;-)).

The main blocker I found is not respecting readOnly: true. I have a schema that has readOnly: true, meaning that the property should only be in responses but not in requests. In my case, I have a POST api to create a resource and a GET api to load the resource, the schema is the same but certain properties (eg created_at) are marked as readOnly. Libninja treats that as a required field even in requests.

Happy to take a look at implementing this if this is something you'd be open to adding / reviewing

[kurtbuilds]

Thank you for raising this issue. Happy to hear more about the other issues you raised (example gen panics; enum support)

Can you provide a minimum OpenAPI spec that uses readonly? It doesn't sound like it'd be too difficult to add support.

Workaround

One workaround idea until this is implemented: I've seen other OpenAPI specs explicitly create two Schemas in the spec. You could write a pre-processing step for the OpenAPI spec to change it before using libninja. Libninja itself has several examples of these pre-processing steps. Depending on the changes needed, sometimes it's been necessary to operate on the Yaml, other times on the OpenAPI rust struct.

Implementation

Have you thought through what the desired codegen looks like? On first pass, I'm imagining something like this:

#[derive(Serialize, Deserialize)
pub struct UserModifiable {
    pub name: String
}

#[derive(Serialize, Deserialize)
pub struct UserResponse {
    pub created_at: DateTime
    #[serde(flatten)]
    pub inner: UserModifiable
}

Open questions that I see:

1. Are the struct names reasonable? Maybe UserFields and User or UserUpdate and User would be simpler naming.
2. Does it make sense to impl Deref to the inner? It's generally frowned upon, but otherwise a lot of field access is behind user.inner.$field. Getters/accessors is another option, but I find those noisy in their own right.
3. This doesn't suit the situation where both Request and Response have different fields.

If you file a PR, happy to review that directly.

[mirek26]

Thanks for the reply - I'll try the workaround as you suggest, updating the OpenAPI rust struct before generation is an interesting idea.

Minimal openapi spec here:

openapi: '3.0.1'
info:
  title: Test Spec
  version: '0.1.0'
paths:
  /object:
    post:
      operationId: create_sth
      requestBody:
        required: true
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Object'
      responses:
        '201': { description: Object created }
    get:
      operationId: fetch_sth
      responses:
        '200':
          description: Successful operation
          content:
            application/json:
              schema:
                type: array
                items:
                  $ref: '#/components/schemas/Object'
components:
  schemas:
    Object:
      type: object
      properties:
        id:
          type: string
          readOnly: true
        name:
          type: string
      required:
        - id
        - name

Per spec:

[...] it MAY be sent as part of a response but SHOULD NOT be sent as part of the request. If the property is marked as readOnly being true and is in the required list, the required will take effect on the response only

Implementation

1. Something like that seems reasonable, I like the idea of calling the main struct just User and the request struct UserUpdate or UserRequest
2. My initial idea was to just generate two independent structs with a one-way conversion function. There would be some duplication but at least it wouldn't impact the usability of the main model in any way.. That solution may also be better if you want to support writeOnly - I'm not sure how broadly used that feature is, but it is in the standard too. Thoughts?

[kurtbuilds]

Sounds good. I'll wait for an update from you on the pre-processing approach.

User and UserRequest sound like good names to me.

Thinking through this, I'm wondering if the actual implementation could just be that pre-processing step. It doesn't require modifying codegen, which is both simpler, and it gets implemented for free for the other languages libninja supports.