allOf - Properties not generated - C#

[gadcam]

Hello,
In the case of an allOf if there are properties directly written in an object along side a $ref the properties are not taken into account in the generated code.
Moving up the properties outside the allOf fix the issue

for schema_name, schema in data['components']['schemas'].items():
    if 'allOf' in schema and schema['allOf']:
        all_of_properties = schema['allOf'][0].get('properties', {})
        schema.setdefault('properties', {}).update(all_of_properties)
        schema['allOf'] = [ref for ref in schema['allOf']
                           if isinstance(ref, dict) and '$ref' in ref]

Seems related to #4325 but not the same exact bug.

Here is an example to illustrate the bug

File 1 - Broken

---
openapi: 3.0.3
servers:
  - url: https://example.com
info:
  title: example
  version: 0.0.1
paths:
  /path:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: "#/components/schemas/ComponentOne"
      responses:
        "201":
          description: Created
          content:
            application/json:
              schema:
                type: string
components:
  schemas:
    ComponentOne:
      title: ComponentOne
      allOf:
        - type: object
          properties:
            propOne:
              type: string
        - $ref: "#/components/schemas/ComponentTwo"
    ComponentTwo:
      properties:
        anotherField:
          type: string

File 2 - Working

---
openapi: 3.0.3
servers:
  - url: https://example.com
info:
  title: example
  version: 0.0.1
paths:
  /path:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: "#/components/schemas/ComponentOne"
      responses:
        "201":
          description: Created
          content:
            application/json:
              schema:
                type: string
components:
  schemas:
    ComponentOne:
      title: ComponentOne
      type: object
      properties:
        propOne:
          type: string
      allOf:
        - $ref: "#/components/schemas/ComponentTwo"
    ComponentTwo:
      properties:
        anotherField:
          type: string

[baywet]

Hi @gadcam,
Thanks for using kiota and for reporting this.
Can you confirm whether you might have inverted the broken and working example please?
The reason I'm asking is because the descriptions for Microsoft Graph contain thousands of instances of your "broken" example" and we'd have caught it by now I assume.
But we don't have instances of your "working" example.

[gadcam]

@baywet Bonjour Vincent,
I am afraid that the samples were correctly labelled. (the Python fix I used can convince you they are)

the descriptions for Microsoft Graph contain thousands of instances of your "broken" example

Then I suppose the problem might involve a more complex structure.
What can I do to provide a better minimal reproducible example ?
Should I write a test ? If so where would be the correct place ?
Thank you a lot for your help!

Here is another example which I just copy pasted from the files and redacted

1. Before the Python script ran

AAA:
      title: AAA
      description: AAA
      allOf:
        - type: object
          properties:
            BBB:
              $ref: '#/components/schemas/DDD'
            CCC:
              $ref: '#/components/schemas/DDD'
        - $ref: '#/components/schemas/DDD'

2. After the Python script ran

    AAA:
      allOf:
      - $ref: '#/components/schemas/DDD'
      description: AAA
      properties:
        CCC:
          $ref: '#/components/schemas/DDD'
        BBB:
          $ref: '#/components/schemas/DDD'
      title: AAA

So not exactly the previous Working example: the order is not exactly the same but the structure is the same.

[baywet]

Thanks for the additional details here.

Just to confirm, I double checked the Microsoft Graph OAI descriptions and here is the general pattern employed for over 2000 models:

    microsoft.graph.user:
      allOf:
        - $ref: '#/components/schemas/microsoft.graph.directoryObject'
        - title: user
          required:
            - '@odata.type'
          type: object
          properties:
            aboutMe:
              type: string
              description: A freeform text entry field for the user to describe themselves. Returned only on $select.
              nullable: true

Which maps to your broken example (except for the order, but hopefully it's not just an ordering thing, worth checking for sanity though)

Tests would go here

kiota/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs

Line 7174 in 972c8f8

public async Task InheritanceWithAllOfWith3Parts()

kiota/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs

Line 7121 in 972c8f8

public async Task InheritanceWithAllOfInBaseType()

Duplicating them to account for the additional scenarios that are currently not tested for would probably be a good first step and might outline something I'm not seeing by just looking at the provided descriptions.

[andreaTP]

it's not just an ordering thing

tested and looks like it's not an ordering thing, at least, in the absence of other edge cases kicking in ( e.g. #4325 )

[baywet]

Started looking into this with #4381 which I believe will fix this one as well, but I need to design a unit test for it.

[baywet]

update: I did manage to fix this unrecognized inheritance case, and remove a bunch of dependencies on schema titles which we didn't want to begin with.
I did run into a regression with types trimming I'll need to look into further

[andreaTP]

I have been attempting to reduce this again, this is what I have:

Common description:

openapi: 3.0.3
servers:
  - url: https://example.com
info:
  title: example
  version: 0.0.1
paths:
  /path:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: "#/components/schemas/ComponentList"
      responses:
        "201":
          description: Created
          content:
            application/json:
              schema:
                type: string
components:
  schemas:
    Component:
      type: object
      properties:
        prop:
          type: string
    ListMeta:
      type: object
      properties:
        size:
          type: number

Adding the definition of ComponentList:

Working:

    ComponentList:
      type: object
      properties:
        items:
          type: array
          items:
            $ref: "#/components/schemas/Component"
      allOf:
        - $ref: "#/components/schemas/ListMeta"

and working as well:

    ComponentList:
      type: object
      allOf:
        - $ref: "#/components/schemas/ListMeta"
        - type: object
          properties:
            items:
              type: array
              items:
                $ref: "#/components/schemas/Component"

results in the expected:

@dataclass
class ComponentList(ListMeta):
    # The items property
    items: Optional[List[Component]] = None

unfortunately switching the order of the allOfs components produces an unexpected result, the entity used by the endpoint becomes ListMeta and the Python hierarchy is mapped "the other way around":

    ComponentList:
      type: object
      allOf:
        - type: object
          properties:
            items:
              type: array
              items:
                $ref: "#/components/schemas/Component"
        - $ref: "#/components/schemas/ListMeta"

@dataclass
class ListMeta(ComponentList):
    # The size property
    size: Optional[float] = None

it's 100% reproducible with kiota 1.12.0 and using a command like:

kiota generate -l python -c PostsClient -n client -d list-issue.yaml -o tmp-list --clean-output --log-level Trace

[microsoft-github-policy-service]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.

[baywet]

(just removed the tag so the bot leaves us alone on this one)

Update: I have paused work on this because of other priorities at the moment, and I'll be out the next two weeks.

[andreaTP]

Thanks for sharing openly your plans @baywet , I have found a few more edge cases and will try to report with minimal reproducers along.

[baywet]

part of what might be making a difference, and looking at #4074 is the presence/absence of type: object at different levels. We'll need to look into this further

[baywet]

Short update: working on #4074 has fixed this as well in #4381
Still running validation on this PR, but things are looking promising so far!