search

TOMP-WG / TOMP-API

Transport Operator to Mobility-as-a-Service Provider-API development for Mobility as a Service
Apache License 2.0
96 stars 41 forks source link

[BUG] GeojsonGeometry definition problem #546

Open adamtomecki opened 3 months ago

adamtomecki commented 3 months ago

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>
2. 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" } ]



3. Exception thrown as mentioned.
edwinvandenbelt commented 3 months ago

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 commented 2 months ago

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

adamtomecki commented 2 months ago

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