[java] Unusable code emitted when a field is overwritten

[andreaTP]

This is clearly an edge case, but I would still like to report it to gather feedback.

I keep finding this pattern:

openapi: 3.0.3
info:
  title: Test API
  description: "A test API"
  version: 0.13.0-SNAPSHOT
paths:
  /api/v1/users:
    get:
      summary: Retrieves a list of users
      description: "Returns a list of all users"
      responses:
        "200":
          description: List of users
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/UserList'
components:
  schemas:
    List:
      required:
      - total
      - items
      type: object
      properties:
        kind:
          type: string
        items:
          type: array
          items:
            type: object
        total:
          format: int32
          description: Total number of entries in the full result set
          type: integer
          nullable: false
        size:
          format: int32
          description: Number of entries per page (returned for fetch requests)
          type: integer
        page:
          format: int32
          description: Current page number (returned for fetch requests)
          type: integer
    UserList:
      allOf:
      - $ref: '#/components/schemas/List'
      - description: List of users
        type: object
        properties:
          items:
            type: array
            items:
              type: string

The intention is clear, declare an abstract List and have concrete instances of it overwriting the items field.
It's pretty clearly not defined behavior, but I keep finding it in various places and it shows that, probably, OpenAPI tooling is handling this (most of those)cases according to the intention (as opposed to the spec).

Removing the items field from the "parent" produces the expected output.

Now, I'm not sure how to proceed here, should we fail instead of silently producing unusable code? Or should we handle this case(since is quite popular) with some kind of overwriting mechanism?

[baywet]

thanks for bringing this up.
As far as I can recall, with the current document, kiota should fail and tell you it doesn't know what to do with the items property in the List base type.
Thanks to the inheritance index it should be fairly easy to check whether a child type has the same property with valid information before failing a skip that property at generation time in the base type instead of failing.
I understand that having it in the base type would make polymorphism easier, but we really don't have enough information to make anything useful out of it. And it'd require more advanced concepts like generics for which the code dom is not designed for, and not all languages support.

[andreaTP]

kiota should fail and tell you it doesn't know what to do with the items property in the List base type.

I just tested the snippet above with the CLI freshly built from main, if this is the expected behavior, is buggy 🐛 🙂 or there is something unexpected going on.

I completely understand the reasons for not doing more advanced "reasoning" and taking "smarter" decisions, completely fine by me as long as the validation rule kicks in.

[baywet]

what's the current behaviour you're experiencing?

[andreaTP]

code is generated, but the items field is unusable as it's not overwritten (e.g. the parent's type is used).

[baywet]

yeah that property should be skipped and a warning should be emitted since we don't have enough information. And resolving that would mean the derived type would have the property now (it's probably not adding it because it's in the parent).
Do you want to have a look at that?

[andreaTP]

I can have a look but I don't guarantee when 🙂 fortunately just got all the relevant PRs merged and this is not directly on my way at the moment.

[baywet]

@andreaTP I'm getting closer to being able to have a look at this one. Just wanted to confirm you're not working on it at this moment?

[andreaTP]

Thanks for checking, I confirm I'm not working on this currently (and tomorrow is liberation day in Pt 🙂 ).
Happy to see you around picking up the challenges @baywet 🙏 !

[baywet]

Actually sorry, I got confused while sorting through issues. Leaving this for now.

[baywet]

@papegaaij started some interesting work in the space with #2836 you might want to checkout @andreaTP

[baywet]

@andreaTP to resume our discussion from the PR here. As far as I can remember, from the PR mentioned in my last comment, any property that "overrides" a parent's type property will get a unique name so it doesn't hide it. During serialization/deserialization, due to the structure of things, it should also take precedence.
I don't think we need a warning anymore/action needs to be taken. Besides maybe verifying this is actually the case. Thoughts?

[andreaTP]

Honestly, I do need to try things out again to assess the current status, I'll try to do it somewhere next week.
Feel free to ping me if my delay increases too much.

[andreaTP]

Ok, I took a look at the situation using kiota at version 1.9.0-preview.202311230001.

any property that "overrides" a parent's type property will get a unique name so it doesn't hide it.

Using the repro description in the initial report I cannot confirm that this is the current behavior.
The items field in the UserList class is not present nor does it have a different name.

I think that this issue has to remain open, at least for now.