Carapacik / swagger_parser

Dart package that takes an OpenApi definition file and generates REST clients based on retrofit and data classes for your project.
https://pub.dev/packages/swagger_parser
MIT License
97 stars 45 forks source link

More protections for Enum Names #165

Closed dickermoshe closed 7 months ago

dickermoshe commented 7 months ago

Adds a list of words that can't be used for Enum names (e.g. values, index...) Doesn't allow - in beginning of Enum name. -name -> minusName

Should it be reverse? -name -> reverseName

Fixes https://github.com/Carapacik/swagger_parser/issues/164 https://github.com/Carapacik/swagger_parser/issues/163

dickermoshe commented 7 months ago

The following schema was broken by these issues:

``` openapi: 3.0.3 info: title: '' version: 0.0.0 (v1) paths: /api/v1/category/: get: operationId: api_v1_category_list parameters: - in: query name: active schema: type: boolean - in: query name: index schema: type: integer - in: query name: name schema: type: string - in: query name: o schema: type: array items: type: string enum: - -index - -name - index - name description: |- Ordering * `name` - Name * `-name` - Name (descending) * `index` - Index * `-index` - Index (descending) explode: false style: form - name: page required: false in: query description: A page number within the paginated result set. schema: type: integer - in: query name: parent schema: type: integer tags: - api security: - tokenAuth: [] - {} responses: '200': content: application/json: schema: $ref: '#/components/schemas/PaginatedCategoryList' description: '' /api/v1/category/{id}/: get: operationId: api_v1_category_retrieve parameters: - in: path name: id schema: type: integer description: A unique integer value identifying this category. required: true tags: - api security: - tokenAuth: [] - {} responses: '200': content: application/json: schema: $ref: '#/components/schemas/Category' description: '' /api/v1/listing/: get: operationId: api_v1_listing_list parameters: - in: query name: isSoldByWeight schema: type: boolean - in: query name: o schema: type: array items: type: string enum: - -regularPrice - -salePrice - regularPrice - salePrice description: |- Ordering * `regularPrice` - Regular Price * `-regularPrice` - Regular Price (descending) * `salePrice` - Sale Price * `-salePrice` - Sale Price (descending) explode: false style: form - name: page required: false in: query description: A page number within the paginated result set. schema: type: integer - in: query name: product schema: type: array items: type: string explode: true style: form - in: query name: regularPrice schema: type: number format: float - in: query name: salePrice schema: type: number format: float - in: query name: store schema: type: integer - in: query name: weightUnit schema: type: string tags: - api security: - tokenAuth: [] - {} responses: '200': content: application/json: schema: $ref: '#/components/schemas/PaginatedListingList' description: '' /api/v1/listing/{id}/: get: operationId: api_v1_listing_retrieve parameters: - in: path name: id schema: type: integer description: A unique integer value identifying this listing. required: true tags: - api security: - tokenAuth: [] - {} responses: '200': content: application/json: schema: $ref: '#/components/schemas/Listing' description: '' /api/v1/non-paginated-listing/: get: operationId: api_v1_non_paginated_listing_list parameters: - in: query name: isSoldByWeight schema: type: boolean - in: query name: o schema: type: array items: type: string enum: - -regularPrice - -salePrice - regularPrice - salePrice description: |- Ordering * `regularPrice` - Regular Price * `-regularPrice` - Regular Price (descending) * `salePrice` - Sale Price * `-salePrice` - Sale Price (descending) explode: false style: form - in: query name: product schema: type: array items: type: string explode: true style: form - in: query name: regularPrice schema: type: number format: float - in: query name: salePrice schema: type: number format: float - in: query name: store schema: type: integer - in: query name: weightUnit schema: type: string tags: - api security: - tokenAuth: [] - {} responses: '200': content: application/json: schema: type: array items: $ref: '#/components/schemas/Listing' description: '' /api/v1/non-paginated-listing/{id}/: get: operationId: api_v1_non_paginated_listing_retrieve parameters: - in: path name: id schema: type: integer description: A unique integer value identifying this listing. required: true tags: - api security: - tokenAuth: [] - {} responses: '200': content: application/json: schema: $ref: '#/components/schemas/Listing' description: '' /api/v1/product/: get: operationId: api_v1_product_list parameters: - in: query name: brand schema: type: integer - in: query name: category schema: type: array items: type: integer explode: true style: form - in: query name: community schema: type: array items: type: string enum: - ALL - BROOKLYN - FIVE_TOWNS - FLATBUSH - KJ - LAKEWOOD - MONSEY - PASSAIC - QUEENS description: |- Community * `LAKEWOOD` - lakewood * `PASSAIC` - passaic * `MONSEY` - monsey * `FLATBUSH` - flatbush * `QUEENS` - queens * `BROOKLYN` - brooklyn * `FIVE_TOWNS` - five_towns * `KJ` - kj * `ALL` - All explode: true style: form - in: query name: measureCount schema: type: number format: float - in: query name: o schema: type: array items: type: string enum: - -listingCount - -measureCount - -name - -pieceCount - listingCount - measureCount - name - pieceCount description: |- Ordering * `name` - Name * `-name` - Name (descending) * `pieceCount` - Piece Count * `-pieceCount` - Piece Count (descending) * `measureCount` - Measure Count * `-measureCount` - Measure Count (descending) * `listingCount` - Popularity * `-listingCount` - Popularity (descending) explode: false style: form - name: page required: false in: query description: A page number within the paginated result set. schema: type: integer - in: query name: pieceCount schema: type: integer - in: query name: q schema: type: string - in: query name: unitOfMeasure schema: type: integer tags: - api security: - tokenAuth: [] - {} responses: '200': content: application/json: schema: $ref: '#/components/schemas/PaginatedProductList' description: '' /api/v1/product/{sku}/: get: operationId: api_v1_product_retrieve parameters: - in: path name: sku schema: type: string description: A unique value identifying this product. required: true tags: - api security: - tokenAuth: [] - {} responses: '200': content: application/json: schema: $ref: '#/components/schemas/Product' description: '' /api/v1/product/sku_lookup/: post: operationId: api_v1_product_sku_lookup_create tags: - api requestBody: content: application/json: schema: $ref: '#/components/schemas/SkuLookupRequestRequest' application/x-www-form-urlencoded: schema: $ref: '#/components/schemas/SkuLookupRequestRequest' multipart/form-data: schema: $ref: '#/components/schemas/SkuLookupRequestRequest' required: true security: - tokenAuth: [] - {} responses: '200': content: application/json: schema: $ref: '#/components/schemas/SkuLookupResponse' description: '' /api/v1/store/: get: operationId: api_v1_store_list parameters: - in: query name: branchName schema: type: string - in: query name: community schema: type: string enum: - brooklyn - five_towns - flatbush - kj - lakewood - monsey - passaic - queens description: |- * `lakewood` - LAKEWOOD * `passaic` - PASSAIC * `monsey` - MONSEY * `flatbush` - FLATBUSH * `queens` - QUEENS * `brooklyn` - BROOKLYN * `five_towns` - FIVE_TOWNS * `kj` - KJ * `lakewood` - LAKEWOOD * `passaic` - PASSAIC * `monsey` - MONSEY * `flatbush` - FLATBUSH * `queens` - QUEENS * `brooklyn` - BROOKLYN * `five_towns` - FIVE_TOWNS * `kj` - KJ - in: query name: name schema: type: string - in: query name: o schema: type: array items: type: string enum: - -branchName - -community - -name - branchName - community - name description: |- Ordering * `name` - Name * `-name` - Name (descending) * `branchName` - Branch Name * `-branchName` - Branch Name (descending) * `community` - Community * `-community` - Community (descending) explode: false style: form - name: page required: false in: query description: A page number within the paginated result set. schema: type: integer tags: - api security: - tokenAuth: [] - {} responses: '200': content: application/json: schema: $ref: '#/components/schemas/PaginatedStoreList' description: '' /api/v1/store/{id}/: get: operationId: api_v1_store_retrieve parameters: - in: path name: id schema: type: integer description: A unique integer value identifying this store. required: true tags: - api security: - tokenAuth: [] - {} responses: '200': content: application/json: schema: $ref: '#/components/schemas/Store' description: '' /schema/: get: operationId: schema_retrieve description: |- OpenApi3 schema for this API. Format can be selected via content negotiation. - YAML: application/vnd.oai.openapi - JSON: application/vnd.oai.openapi+json parameters: - in: query name: format schema: type: string enum: - json - yaml - in: query name: lang schema: type: string enum: - af - ar - ar-dz - ast - az - be - bg - bn - br - bs - ca - ckb - cs - cy - da - de - dsb - el - en - en-au - en-gb - eo - es - es-ar - es-co - es-mx - es-ni - es-ve - et - eu - fa - fi - fr - fy - ga - gd - gl - he - hi - hr - hsb - hu - hy - ia - id - ig - io - is - it - ja - ka - kab - kk - km - kn - ko - ky - lb - lt - lv - mk - ml - mn - mr - ms - my - nb - ne - nl - nn - os - pa - pl - pt - pt-br - ro - ru - sk - sl - sq - sr - sr-latn - sv - sw - ta - te - tg - th - tk - tr - tt - udm - uk - ur - uz - vi - zh-hans - zh-hant tags: - schema security: - tokenAuth: [] - {} responses: '200': content: application/vnd.oai.openapi: schema: type: object additionalProperties: {} application/yaml: schema: type: object additionalProperties: {} application/vnd.oai.openapi+json: schema: type: object additionalProperties: {} application/json: schema: type: object additionalProperties: {} description: '' components: schemas: Brand: type: object properties: id: type: integer readOnly: true name: type: string maxLength: 255 required: - id - name BundleDeal: type: object properties: id: type: integer readOnly: true quantity: type: integer price: type: number format: double required: - id - price - quantity Category: type: object properties: id: type: integer readOnly: true name: type: string maxLength: 255 index: type: integer parent: allOf: - $ref: '#/components/schemas/SingleCategory' readOnly: true nullable: true active: type: boolean icon: type: string format: uri nullable: true maxLength: 200 selectedIcon: type: string format: uri nullable: true maxLength: 200 image: type: string format: uri nullable: true maxLength: 200 menuBackgroundImage: type: string format: uri nullable: true maxLength: 200 mobileImage: type: string format: uri nullable: true maxLength: 200 hasDirectProdcuts: type: boolean isSimpleParent: type: boolean subcategories: type: array items: $ref: '#/components/schemas/SingleCategory' readOnly: true required: - id - index - name - parent - subcategories CommunityEnum: enum: - lakewood - passaic - monsey - flatbush - queens - brooklyn - five_towns - kj type: string description: |- * `lakewood` - LAKEWOOD * `passaic` - PASSAIC * `monsey` - MONSEY * `flatbush` - FLATBUSH * `queens` - QUEENS * `brooklyn` - BROOKLYN * `five_towns` - FIVE_TOWNS * `kj` - KJ Listing: type: object properties: id: type: integer readOnly: true store: allOf: - $ref: '#/components/schemas/Store' readOnly: true product: allOf: - $ref: '#/components/schemas/Product' readOnly: true regularPrice: type: number format: double salePrice: type: number format: double nullable: true url: type: string format: uri maxLength: 200 isSoldByWeight: type: boolean weightUnit: type: string nullable: true maxLength: 255 bundleDeals: type: array items: $ref: '#/components/schemas/BundleDeal' readOnly: true required: - bundleDeals - id - isSoldByWeight - product - regularPrice - store - url PaginatedCategoryList: type: object properties: count: type: integer example: 123 next: type: integer nullable: true example: '2' previous: type: integer nullable: true example: '1' results: type: array items: $ref: '#/components/schemas/Category' PaginatedListingList: type: object properties: count: type: integer example: 123 next: type: integer nullable: true example: '2' previous: type: integer nullable: true example: '1' results: type: array items: $ref: '#/components/schemas/Listing' PaginatedProductList: type: object properties: count: type: integer example: 123 next: type: integer nullable: true example: '2' previous: type: integer nullable: true example: '1' results: type: array items: $ref: '#/components/schemas/Product' PaginatedStoreList: type: object properties: count: type: integer example: 123 next: type: integer nullable: true example: '2' previous: type: integer nullable: true example: '1' results: type: array items: $ref: '#/components/schemas/Store' Product: type: object properties: sku: type: string maxLength: 255 name: type: string maxLength: 255 imageUrl: type: string format: uri nullable: true maxLength: 200 measureCount: type: number format: double nullable: true pieceCount: type: integer category: allOf: - $ref: '#/components/schemas/SingleCategory' readOnly: true nullable: true brand: allOf: - $ref: '#/components/schemas/Brand' readOnly: true nullable: true unitOfMeasure: allOf: - $ref: '#/components/schemas/UnitOfMeasure' readOnly: true nullable: true listingCount: type: integer nullable: true description: The number of listings for this product onSale: type: boolean averagePrice: type: number format: double nullable: true description: The average price of the product required: - brand - category - name - pieceCount - sku - unitOfMeasure SingleCategory: type: object properties: id: type: integer readOnly: true name: type: string maxLength: 255 index: type: integer parent: type: integer nullable: true active: type: boolean icon: type: string format: uri nullable: true maxLength: 200 selectedIcon: type: string format: uri nullable: true maxLength: 200 image: type: string format: uri nullable: true maxLength: 200 menuBackgroundImage: type: string format: uri nullable: true maxLength: 200 mobileImage: type: string format: uri nullable: true maxLength: 200 hasDirectProdcuts: type: boolean isSimpleParent: type: boolean required: - id - index - name SkuLookupRequestRequest: type: object properties: sku: type: string minLength: 1 required: - sku SkuLookupResponse: type: object properties: product: allOf: - $ref: '#/components/schemas/Product' readOnly: true nullable: true similar: type: array items: $ref: '#/components/schemas/Product' readOnly: true required: - product - similar Store: type: object properties: id: type: integer readOnly: true name: type: string maxLength: 255 branchName: type: string nullable: true maxLength: 255 community: $ref: '#/components/schemas/CommunityEnum' url: type: string format: uri maxLength: 200 required: - branchName - community - id - name - url UnitOfMeasure: type: object properties: id: type: integer readOnly: true short_name: type: string maxLength: 255 long_name: type: string maxLength: 255 plural_name: type: string nullable: true maxLength: 255 required: - id - long_name - short_name securitySchemes: tokenAuth: type: apiKey in: header name: Authorization description: Token-based authentication with required prefix "Token" ```
StarProxima commented 7 months ago

@dickermoshe Thank you for your input, we appreciate it!

LGTM! However, could you please try to write a E2E test to verify that enums value names are generated correctly, for a specification that only contains enums whose value names might have problems?

dickermoshe commented 7 months ago

Done!

dickermoshe commented 7 months ago

In the future I would recommend that the regexes also apply to Enum names. API Client generation is really tricky, it would be awesome if users could patch issue themselves with regex rather than picking another package / contributing fix for their very specific issue.

StarProxima commented 7 months ago

In fact, there can be many such problems and adding separate tools to enable you to fix specific problems may not be the best idea.

The best option here is to automatically fix the specification to suit the needs of the developer and the parser using dart. This is simpler than it may show at first glance, and allows you to automatically fix all specific issues when the file changes / when the spec is retrieved from the url.

But, it's also worth keeping in mind that this only applies to hard to implement/specific problems. For commonly occurring problems, it is worth considering fixing them at the parser level.

StarProxima commented 7 months ago

Thank you for your work, it will be merged after @Carapacik review.

dickermoshe commented 7 months ago

Thanks!