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

Fix decoding error for searchRecentTweets if there is no data #54

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Tunous
Copy link
Contributor

@Tunous Tunous commented Oct 2, 2022

When calling searchRecentTweets method the response from Twitter might not contain data property which results in decoding error for TwitterAPIDataIncludesAndMeta.

Example response:

{
  "meta": {
    "result_count": 0
  }
}

This pull request makes data property optional which resolves decoding issue. But perhaps a better idea would be to introduce a separate type for cases where data is an Array and instead of optional return empty? Or even change Resource type of TwitterAPIDataIncludesAndMeta to always expect array? Looking at the tests there are no cases where data is of non-array type.

@daneden
Copy link
Owner

daneden commented Oct 3, 2022

Oh this is an odd one; the reason you won't find any tests with TwitterAPIDataIncludesAndMeta where Resource isn't an array is that meta typically indicates paginated responses. Search results are no exception, but it’s interesting that Twitter returns an object with no data field—one might expect the following instead:

{
  "data": [],
  "meta": {
    "result_count": 0
  }
}

I like your suggestion of changing Resource for TwitterAPIDataIncludesAndMeta to always expect an array e.g. <Resource: [Codable]> and think that might be better than having data be optional.

@Tunous
Copy link
Contributor Author

Tunous commented Oct 3, 2022

I've changed my code to make data a required array of Resource. I think that should be enough.

I tried to make Resource a Sequence but then I'm not really sure how to create an empty sequence in added initializer.

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

Successfully merging this pull request may close these issues.

None yet

2 participants