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

[objc] Add casts that avoid method resolution errors for count #1340

Merged
merged 1 commit into from Oct 9, 2015
Merged

[objc] Add casts that avoid method resolution errors for count #1340

merged 1 commit into from Oct 9, 2015

Conversation

ches
Copy link
Contributor

@ches ches commented Oct 6, 2015

Because the data that we're deserializing is of type id (essentially untyped), it's possible to have method resolution clashes without explicitly casting here once we've sufficiently parsed a type. I had this issue with a pagination container model, for instance, which has a field named count that conflicts with the property of the same name on NSArray or NSDictionary, preventing the code from compiling until I made this change.

Because the `data` that we're deserializing is of type `id` (essentially
untyped), it's possible to have method resolution clashes without
explicitly casting here once we've parsed a type. I had this issue with
a pagination container model, for instance, which has a field named
`count` that conflicts with the property of the same name on `NSArray`
or `NSDictionary`.
@ches ches changed the title [objc] Add casts that avoid method resolution errors [objc] Add casts that avoid method resolution errors for count Oct 6, 2015
@wing328
Copy link
Contributor

wing328 commented Oct 6, 2015

@ches thanks for the PR. Do you mind sharing the spec for reproducing the issue?

@ches
Copy link
Contributor Author

ches commented Oct 6, 2015

The actual spec is private and is rather complicated to reduce to a minimal example (the API is JSON API-compliant and the Swagger spec models basically the entire JSON Schema of the spec's container structures, etc.), but I think this part is enough to create a reproduction case:

definitions:
  PaginatedResourcesResponse:
    description: A successful collection response with pagination metadata.
    allOf:
      - $ref: '#/definitions/SuccessResponseCollection'
      - properties:
          meta:
            type: object
            properties:
              pagination:
                $ref: '#/definitions/PaginationMetadata'

  PaginationMetadata:
    properties:
      count:
        description: Number of results in current page.
        type: integer
      total:
        description: Total number of results available.
        type: integer
      total_pages:
        type: integer
      per_page:
        type: integer
      current_page:
        type: integer
      links:
        type: object
        properties:
          next:
            title: URL for the next page of results.
            type: string
            format: url
          previous:
            title: URL for the previous page of results.
            type: string
            format: url

Nevermind the elided SuccessResponseCollection, substitute anything for that. The salient part is that PaginationMetadata has a property called count. Without this patch, data is an ObjC id, and the compiler can't figure out which count method should be dispatched, it clashes with the count properties on NSArray, NSDictionary, etc.

@wing328
Copy link
Contributor

wing328 commented Oct 8, 2015

@ches thanks for the details in reproducing the issue. I'll use it to confirm and let you know if I've further questions on the PR.

@wing328 wing328 added this to the v2.1.4 milestone Oct 8, 2015
@ches
Copy link
Contributor Author

ches commented Oct 9, 2015

cc @geekerzp perhaps who has done much of the ObjC client work it seems.

@wing328
Copy link
Contributor

wing328 commented Oct 9, 2015

Was able to repeat the issue and integration test in your branch looks good:

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 27.464 s
[INFO] Finished at: 2015-10-09T16:40:35+08:00
[INFO] Final Memory: 11M/245M
[INFO] ------------------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 27.464 s
[INFO] Finished at: 2015-10-09T16:40:35+08:00
[INFO] Final Memory: 11M/245M
[INFO] ------------------------------------------------------------------------

wing328 added a commit that referenced this pull request Oct 9, 2015
[objc] Add casts that avoid method resolution errors for `count`
@wing328 wing328 merged commit d95660a into swagger-api:master Oct 9, 2015
@ches
Copy link
Contributor Author

ches commented Oct 9, 2015

Thanks! I think I will have some more fixes for ObjC on the way soon 😃

@wing328
Copy link
Contributor

wing328 commented Oct 10, 2015

Thanks @ches

To run the ObjC integration test locally, please execute the following commands in the project root directory:

./bin/objc-petstore.sh
cd samples/client/petstore/objc/SwaggerClientTests 
mvn integration-test -rf :ObjcPetstoreClientTests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants