Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] GeojsonGeometry definition problem #546

Open
adamtomecki opened this issue Jun 12, 2024 · 3 comments · May be fixed by #552
Open

[BUG] GeojsonGeometry definition problem #546

adamtomecki opened this issue Jun 12, 2024 · 3 comments · May be fixed by #552
Assignees
Labels
WT1 To be discussed at the next meeting
Milestone

Comments

@adamtomecki
Copy link

API Version

1.6

Summary

When I generate POJOs using openapitools generator in version 7.4.0 then for GeojsonGeometry object there is no subtypes defined and 'coordinates' property is an empty GeojsonGeometryCoordinates class without any properties inside.
It causes issue when I call e.g. GET operator/stations endpoint:

Caused by: com.fasterxml.jackson.databind.exc.InvalidTypeIdException: Could not resolve type id 'Point' as a subtype of org.tompapiclient.tompapi.model.GeojsonGeometry: known type ids = [geojsonGeometry] (for POJO property 'stationArea')

It happens because of wrongly defined GeojsonGeometry object, with discriminator on wrong level, without mapping between 'type' enum and subtypes.

Expected Behavior

There should be GeojsonGeometry object generated with proper subtypes (GeojsonPoint, GeojsonLine, GeojsonPolygon, GeojsonMultiPolygon) and every subtype class needs to have 'coordinates' property, since it's specific for the subtype.

Current Behavior

GeojsonGeometry object is generated without sybtypes, with two properties: 'type' and 'coordinates', where 'coordinates' is another empty object (GeojsonGeometryCoordinates).

Possible Solution

The following solution works as expected and doesn't break geojson standard:

    geojsonGeometry:
      type: object
      discriminator:
        propertyName: type
        mapping:
          Point: "#/components/schemas/geojsonPoint"
          LineString: "#/components/schemas/geojsonLine"
          Polygon: "#/components/schemas/geojsonPolygon"
          MultiPolygon: "#/components/schemas/geojsonMultiPolygon"
      description: geoJSON geometry
      required:
        - type
      properties:
        type:
          type: string
          enum: [
            "Point",
            "LineString",
            "Polygon",
            "MultiPolygon"
          ]

    basePoint:
      type: array
      minItems: 2
      maxItems: 2
      items:
        type: number
        format: float
        minimum: 0.0
      example: [ 4.53432, 55.324523 ]

    geojsonPoint:
      type: object
      description: Geojson Coordinate
      allOf:
        - $ref: "#/components/schemas/geojsonGeometry"
        - type: object
          properties:
            coordinates:
              $ref: "#/components/schemas/basePoint"

    geojsonLine:
      type: object
      description: An array of WGS84 coordinate pairs
      allOf:
        - $ref: "#/components/schemas/geojsonGeometry"
        - type: object
          properties:
            coordinates:
              type: array
              example: [ [ 6.169639, 52.253279 ], [ 6.05623, 52.63473 ] ]
              items:
                $ref: "#/components/schemas/basePoint"

    geojsonPolygon:
      type: object
      description: geojson representation of a polygon. First and last point must be equal. See also https://geojson.org/geojson-spec.html#polygon and example https://geojson.org/geojson-spec.html#id4. The order should be lon, lat [[[lon1, lat1], [lon2,lat2], [lon3,lat3], [lon1,lat1]]], the first point should match the last point.
      allOf:
        - $ref: "#/components/schemas/geojsonGeometry"
        - type: object
          properties:
            coordinates:
              type: array
              example: [ [ [ 1.0, 1.0 ], [ 0.0, 1.0 ], [ 0.0, 0.0 ], [ 1.0,0.0 ], [ 1.0, 1.0 ] ] ]
              items:
                type: array
                example: [ [ 6.169639, 52.253279 ], [ 6.05623, 52.63473 ] ]
                items:
                  $ref: "#/components/schemas/basePoint"

    geojsonMultiPolygon:
      type: object
      description: geojson representation of a multi polygon. See also https://geojson.org/geojson-spec.html#multipolygon
      allOf:
        - $ref: "#/components/schemas/geojsonGeometry"
        - type: object
          properties:
            coordinates:
              type: array
              example: [ [ [ [ 1.0, 1.0 ], [ 0.0, 1.0 ], [ 0.0, 0.0 ], [ 1.0,0.0 ], [ 1.0, 1.0 ] ] ] ]
              items:
                type: array
                example: [ [ [ 1.0, 1.0 ], [ 0.0, 1.0 ], [ 0.0, 0.0 ], [ 1.0,0.0 ], [ 1.0, 1.0 ] ] ]
                items:
                  type: array
                  example: [ [ 6.169639, 52.253279 ], [ 6.05623, 52.63473 ] ]
                  items:
                    $ref: "#/components/schemas/basePoint"
		  

Steps to Reproduce

  1. Generate POJOs from TOMP-API.yaml in version 1.6 using openapitools generator in version 7.4.0. Plugin definition from pom:

	  <plugin>
                <groupId>org.openapitools</groupId>
                <artifactId>openapi-generator-maven-plugin</artifactId>
                <version>7.4.0</version>
                <executions>
                    <execution>
                        <id>tompapi</id>
                        <goals>
                            <goal>generate</goal>
                        </goals>
                        <configuration>
                            <generatorName>java</generatorName>
                            <library>webclient</library>
                            <inputSpec>
                                ${project.basedir}/src/main/resources/openapi/tompapi/tompapi-1-6.yaml
                            </inputSpec>
                            <skipIfSpecIsUnchanged>true</skipIfSpecIsUnchanged>
                            <generateApis>true</generateApis>
                            <generateApiDocumentation>false</generateApiDocumentation>
                            <generateApiTests>false</generateApiTests>
                            <generateModels>true</generateModels>
                            <generateModelDocumentation>false</generateModelDocumentation>
                            <generateModelTests>false</generateModelTests>
                            <output>${project.build.directory}/generated-sources</output>
                            <modelPackage>${project.groupId}.${project.artifactId}.tompapi.model</modelPackage>
                            <apiPackage>${project.groupId}.${project.artifactId}.tompapi.api</apiPackage>
                            <configOptions>
                                <openApiNullable>false</openApiNullable>
                                <useSpringBoot3>true</useSpringBoot3>
                                <interfaceOnly>true</interfaceOnly>
                                <useJakartaEe>true</useJakartaEe>
                            </configOptions>
                        </configuration>
                    </execution>
                </executions>
            </plugin>
	
  1. Try to use the POJOs to get data from GET /operator/stations endpoint. Result data example:
[
    {
        "stationId": "XX:Y:12345678",
        "name": "IslandCentral",
        "contactPhone": "string",
        "coordinates": {
            "lng": 6.169639,
            "lat": 52.253279,
            "alt": 0
        },
        "physicalAddress": {
            "streetAddress": "examplestreet18,2ndfloor,18-B33",
            "street": "string",
            "houseNumber": 0,
            "houseNumberAddition": "string",
            "addressAdditionalInfo": "string",
            "areaReference": "Smallcity,Pinetreecounty",
            "city": "string",
            "province": "string",
            "state": "string",
            "postalCode": "string",
            "country": "NL"
        },
        "crossStreet": "onthecornerwithSecondaryRoad",
        "regionId": "string",
        "stationArea": {
            "type": "Point",
            "coordinates": [
                4.53432,
                55.324523
            ]
        },
        "parkingType": "PARKING_LOT",
        "isVirtual": true,
        "isValetStation": true,
        "isChargingStation": true,
        "parkingHoop": true,
        "isInstalled": true,
        "isRenting": true,
        "isReturning": true,
        "capacity": 0,
        "assetTypeCapacity": {
            "child-bike-01": 3,
            "general-bike": 18
        },
        "assetsAvailable": 0,
        "assetTypesAvailable": {
            "child-bike-01": 1,
            "general-bike": 3
        },
        "docksAvailable": 0,
        "assetDocksAvailable": {
            "child-bike-01": 1,
            "general-bike": 3
        },
        "rentalMethods": [
            "CREDITCARD",
            "PAYPASS",
            "APPLEPAY"
        ],
        "rentalMethodOther": "iDEAL"
    }
]
  1. Exception thrown as mentioned.
@edwinvandenbelt
Copy link
Collaborator

Hello Adam! We're going to pick up your issue. First, we're going to investigate your solution, since it looks good. I didn't know the 'mapping' construct. On first glance, it might look like a mis-indention in the current situation. Could you easily find out if this is the case? (Add 4 spaces in front of the discriminator labels). Btw I like your solution, I'm seriously going to look at it, use it in V2.0 (or, if it won't break things, in v1.6).

@edwinvandenbelt edwinvandenbelt added the WT1 To be discussed at the next meeting label Jun 20, 2024
@edwinvandenbelt edwinvandenbelt self-assigned this Jun 20, 2024
@edwinvandenbelt edwinvandenbelt added this to the 1.6 milestone Jun 20, 2024
@edwinvandenbelt edwinvandenbelt moved this from To do to In progress in WT1 Kanban Jun 20, 2024
@edwinvandenbelt edwinvandenbelt moved this from In progress to In review in WT1 Kanban Jul 10, 2024
@edwinvandenbelt
Copy link
Collaborator

Patched it! Your solution was very good! I didn't know the 'mapping' construct. I will apply it in v2.0 as well! Could you test it? @adamtomecki

@edwinvandenbelt edwinvandenbelt linked a pull request Jul 11, 2024 that will close this issue
@adamtomecki
Copy link
Author

@edwinvandenbelt Hi Edwin, it looks very good, thank you for resolving that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WT1 To be discussed at the next meeting
Projects
Status: In review
Status: In review
Development

Successfully merging a pull request may close this issue.

2 participants