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

Implement "fields" parameter from the YouTube Data API #429

Merged
merged 7 commits into from Mar 31, 2019

Conversation

@afrmtbl
Copy link
Contributor

commented Mar 23, 2019

Allows filtering JSON responses from the Invidious API

For example, to only include the title and videoId from the JSON response, use:
?fields=title,videoId

Asterisks/wildcards aren't supported as I didn't see the point.
To include all fields from an object, simply specify the entire object.

Reference: https://developers.google.com/youtube/v3/getting-started#fields

@afrmtbl

This comment has been minimized.

Copy link
Contributor Author

commented Mar 23, 2019

On the server side if there's an error with parsing fields, it will only be logged using puts. Is there something else it should be doing?

@omarroth

This comment has been minimized.

Copy link
Owner

commented Mar 23, 2019

Very nice, thank you! I left a couple comments, but otherwise this looks ready to merge 🎉 .

If the error code is 400, then that indicates that the client sent a malformed request, which isn't something that needs to be logged.

Usually only 500 errors should log any information, in which case it should be:

logger.write("MESSAGE\n")
@omarroth

This comment has been minimized.

Copy link
Owner

commented Mar 23, 2019

How are fields applied to arrays? For example, how would I retrieve videoThumbnails/url from the following:

{
  "videoId": "CvFH_6DNRCY",
  "videoThumbnails": [
    {
      "quality": "maxres",
      "url": "https://invidio.us/vi/CvFH_6DNRCY/maxres.jpg",
      "width": 1280,
      "height": 720
    },
    {
      "quality": "maxresdefault",
      "url": "https://i.ytimg.com/vi/CvFH_6DNRCY/maxresdefault.jpg",
      "width": 1280,
      "height": 720
    }
  ]
}

Is this possible with the fields API?

@afrmtbl

This comment has been minimized.

Copy link
Contributor Author

commented Mar 23, 2019

The log on 400 was removed.
As for filtering inside arrays, looks like the YouTube API does support it:

YouTube API Example URL:
https://www.googleapis.com/youtube/v3/commentThreads?videoId=Kb3cwTfSOjw&part=snippet&fields=items/snippet/canReply&key=AIzaSyBBdCo-KyDeWBHkR2JrGdeL7tyrMq3z1-4

Currently on Invidious it returns:

{"error":"Expected Hash for #[]?(key : String), not Array(JSON::Any)"}

The test URL being: http://0.0.0.0:3000/api/v1/comments/Kb3cwTfSOjw?fields=comments/author

I wouldn't mind trying to implement it.

@afrmtbl

This comment has been minimized.

Copy link
Contributor Author

commented Mar 30, 2019

Filtering inside arrays should now work 🎉

To get only url from videoThumbnails in your example above, simply use either of the following:

?fields=videoThumbnails/url
?fields=videoThumbnails(url)

Getting multiple properties from an array is the same as a normal JSON object:

?fields=videoThumbnails(url,width,height)
?fields=videoThumbnails/url,videoThumbnails/width,videoThumbnails/height

I also improved error messages.
?fields=videoThumbnails/url/invalid should now return:

{"error":"Can't filter '/vi/CvFH_6DNRCY/maxres.jpg' by [\"invalid\"]"}

It works fine as is, but I was wondering if I could simplify this part?
https://github.com/afrmtbl/invidious/blob/ccaa4d6ed824130b680715aa6412a7753de8a310/src/invidious/jsonfilter.cr#L175-L180

I don't really understand why JSONSkeleton.new doesn't work, but SkeletonValue.new does.

Thanks!

src/invidious/jsonfilter.cr Outdated Show resolved Hide resolved
src/invidious/jsonfilter.cr Outdated Show resolved Hide resolved
src/invidious/jsonfilter.cr Outdated Show resolved Hide resolved
src/invidious/jsonfilter.cr Outdated Show resolved Hide resolved
src/invidious/jsonfilter.cr Outdated Show resolved Hide resolved
src/invidious/jsonfilter.cr Outdated Show resolved Hide resolved
src/invidious/jsonfilter.cr Outdated Show resolved Hide resolved
@omarroth

This comment has been minimized.

Copy link
Owner

commented Mar 30, 2019

Looks great! I suggested a couple changes, very excited to merge this.

@omarroth

This comment has been minimized.

Copy link
Owner

commented Mar 31, 2019

LGTM 👍

@omarroth omarroth merged commit a7723e6 into omarroth:master Mar 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.