Skip to content
New issue

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

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

Already on GitHub? # to your account

LeagueTable data is not decoded properly #6

Closed
acche opened this issue Oct 21, 2016 · 22 comments · Fixed by #5
Closed

LeagueTable data is not decoded properly #6

acche opened this issue Oct 21, 2016 · 22 comments · Fixed by #5
Assignees
Labels
Milestone

Comments

@acche
Copy link

acche commented Oct 21, 2016

Request from SoccerSeasonLeagueTableRequest, and find some football team name return empty.For instance request from http://api.football-data.org/v1/competitions/427/leagueTable, the teams include "Bristol City" ,"Brentford", "Reading" will return empty, but check that API on the browser they have value. So what's the problem?

@acche acche changed the title Some Team name return null value in league table Some Team name return empty value in league table Oct 21, 2016
@icedream icedream added the bug label Oct 23, 2016
@icedream icedream added this to the v0.1.0 milestone Oct 23, 2016
@icedream icedream self-assigned this Oct 23, 2016
@icedream
Copy link
Owner

Will check that asap.

@icedream
Copy link
Owner

icedream commented Oct 23, 2016

It seems that the Football Data API documentation mismatches the data that is actually returned from the API server. The LeagueTable is supposed to contain a field "team" in each standing information which contains the team name, however the actual data has that field renamed to "teamName". I'll fix that in my implementation.

(In fact a lot of the real data does not match the descriptions of the fields given in the documentation... I should probably update all data structures in the library at this point.)

@icedream icedream changed the title Some Team name return empty value in league table LeagueTable data is not decoded properly Oct 23, 2016
@acche
Copy link
Author

acche commented Nov 2, 2016

I just test develop branch, and I found the "TeamName" not return any value to the API request, and modify it to "Team", and still some football team name did not return any value. The problem still happen like the previous question described.

@icedream
Copy link
Owner

icedream commented Nov 4, 2016

Yeah I already have seen that in my own local tests as well... could be an issue with the response JSON from the API server being wrongly decoded but I'm unsure yet.

@acche
Copy link
Author

acche commented Nov 9, 2016

Did you try to test the API request with any other server language?

@acche
Copy link
Author

acche commented Dec 16, 2016

Hey Master, do you have any solution for this issue? I am still running on your library.

@icedream
Copy link
Owner

Sorry for idling on the project but unfortunately I'm completely busy with work. I didn't manage to figure out how to fix this issue yet and I'm currently hoping for someone else to chime in and help me with this.

@Kavit900
Copy link
Contributor

Kavit900 commented Aug 1, 2017

Hi guys, I really liked your API @icedream , can I look into this part, if I figure out something then I could send a pull request for my branch, what do you think ?

@icedream
Copy link
Owner

icedream commented Aug 1, 2017

@Kavit900 That would be awesome! 👍

@Kavit900
Copy link
Contributor

Kavit900 commented Aug 2, 2017

Cool then forking the branch, this could be a great weekend project 👍

@Kavit900
Copy link
Contributor

Kavit900 commented Aug 2, 2017

@icedream can you tell me how to do test the code, like how do you run the Example method present in example_test.go, thanks and sorry new to this part

@Kavit900
Copy link
Contributor

Kavit900 commented Aug 2, 2017

One thing I noticed here is that there is no struct in api_types.go that would decode a team information and LeagueTableofSoccerSeason returns a struct of type SoccerSeason and that struct doesn't contain anything to decode the team info, may be my understanding is wrong

@icedream
Copy link
Owner

icedream commented Aug 2, 2017

@Kavit900 Are you on the master or the develop branch? I remember the master branch having some wrong type definitions that should be fixed on develop.

All current code that should be edited is on the develop branch and that will be part of the next release. All pull requests should be done against that branch. Unfortunately a large part of the code base currently has no unit tests going, theoretically you could take the example code and rewrite it as a test function to fetch specific expected information (without an API key) and see if the decoded data matches, then that code can be run as part of automated testing using go test.

@Kavit900
Copy link
Contributor

Kavit900 commented Aug 2, 2017

thanks @icedream ya I am on the master branch that's why I was having trouble understanding it, will checkout develop branch then ... got it ... I will do that 👍

@Kavit900
Copy link
Contributor

Kavit900 commented Aug 3, 2017

So when I changed the name from TeamName to only Team then I was able to retrieve the names of some of the clubs that means something is wrong with the decoding part, I am still trying to fix this but if I am not able to then I would like to propose a hacky way of getting the names of all the teams, so the crestURI is getting updated fine and so I was thinking that we can pull out the TeamName from the crest URI, I know it's not the right way but could be a solution for the time being, just a thought :)

@icedream
Copy link
Owner

icedream commented Aug 4, 2017

I agree that it's not a clean solution, I'm still wondering why the decoding doesn't work properly just in that one case. That workaround would definitely need to be documented at the very least if it makes its way into the next version. Hm.

@Kavit900
Copy link
Contributor

Kavit900 commented Aug 4, 2017

Ya I agree with you @icedream, I wanted to ask one thing, I am not pro with how the json.NewDecoder() method works in go but the api has attribute called teamName or goalsAgainst, how does it automatically know to map goalsAgainst to GoalsAgainst attribute present in the struct or to map teamName to TeamName, is there a place where we can define a rule for the mapping ?

@Kavit900
Copy link
Contributor

Kavit900 commented Aug 4, 2017

Ok Found something more into this and found out why the decoding is not working properly that's because for this league [http://api.football-data.org/v1/competitions/427/leagueTable] , if you see the data for Reading or Leeds United, they don't have all the values in place, like their CrestURI is empty and for these only the decoding is not working properly, interesting .... working on a hacky fix, will try to send a pull request by today end :)

@icedream
Copy link
Owner

icedream commented Aug 4, 2017

There's naming conventions that generally allow the decoder to guess the right mapping (see https://blog.golang.org/json-and-go or the excerpt from it below).

How does Unmarshal identify the fields in which to store the decoded data? For a given JSON key "Foo", Unmarshal will look through the destination struct's fields to find (in order of preference):

  • An exported field with a tag of "Foo" (see the Go spec for more on struct tags),
  • An exported field named "Foo", or
  • An exported field named "FOO" or "FoO" or some other case-insensitive match of "Foo".

For explicit mapping, the forementioned struct tags can be used (`json:"Foo"`).

Looking forward to the pull request!

@icedream
Copy link
Owner

I'll have time this evening to finally test the fix properly. I'll let you know if it's fixed and if it is, I'm immediately merging this with the master branch.

@icedream icedream mentioned this issue Aug 10, 2017
@Kavit900
Copy link
Contributor

Sounds Awesome @icedream

icedream added a commit that referenced this issue Aug 10, 2017
This removes the request header `X-Response-Control` as it was set to `minified` which caused the API to send a different set of data back which did not decode back to the given structure properly.

In the future, there are plans to give the developer full control over the output by providing other variants of the struct.

Fixes #6.
@icedream
Copy link
Owner

icedream commented Aug 10, 2017

As you can see from the commit, I finally found out what caused the returned data to not be decoded properly. It was simply because the API in fact did not send this information back or used shorter variants of the information as the library sent the X-Response-Control: minified header. First of all, it was a mistake to hardcode this and second, the documentation was unclear about how much shortage there would be in the information being sent back. I assumed it would only trim down the _links metadata and additional whitespace in the JSON but it seems it actually strips the meta information itself down.

Took me wayyyy too long to figure out.

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

Successfully merging a pull request may close this issue.

3 participants