-
Notifications
You must be signed in to change notification settings - Fork 106
API 2.0 #862
Comments
Sounds very nice :) Is it really necessary to only sync the changes of the feeds and folders? At least in my experience syncing feeds and folders is super fast anyway. |
@jangernert We used to update items by adding new items to the database so we could not use the id to identify the article (also think about the autopaging, the article would have been at the bottom) and starring items should always hit the right article As for syncing feeds/folders: there are people with 200 feeds and 20 folders, dont ask me why :D Since almost all API users are mobile apps, we want to reduce the payload as much as possible |
The one thing I have found most cumbersome is to sync the items of a folder. First the feeds of that folder need to be retrieved and then iterated over resulting in several server calls. On the other hand adding a folder id to each item is maybe not so smart either. |
Sounds cool to get a new API :).
Items and feeds could/should still have the feedId/folderId. This would reduce initial load as items could be loaded upon folder/feed-request and not all at once.. |
@mk-conn I like your idea. |
+1 For any ideas which support removing of weird parameters (guidHash, feedId) in marking multiple items as (un)starred. This is just nonsense. When you mark as (un)read, you use message ID, which is correct (what else identificator you should use, right?). When you (un)star you suddenly use guidHash. Wow. So any client had to keep three values for each message instead of only one. Looking forward to 2.0 API :), great work guys. |
Btw: I noticed that currently it is not possible to subscribe to feeds which require authentication via API. No idea if ownCloud News supports these kind of feeds in general (I am not subscribed to any). But once ownCloud News does support them, it would be nice to also be able subscribe to them via the API :) |
@jangernert sure ;) |
The thing about feeds that require authentication is that we would need to save the credentials in plain text (which is not desirable at all) and OAuth is probably not very common. |
@BernhardPosselt Is it not possible to store credentials encrypted somehow? |
Nope, how would the update work ;)? |
Hm, maybe I don't really get the problem.. but during/before update just decrypt credentials? |
If you can just decrypt them then you've achieved nothing but more complexity ;) |
I've had some time to work on the spec which is no located in the docs folder, feedback would be awesome: |
Ok, finished the spec, please comment on https://github.com/owncloud/news/tree/master/docs/developer/External-Api.md |
Great work, the new API looks very promising. Though one point irritates me a bit. The "data" wrapper object seems to be unnecessary and complicates parsing. If you want to make the API even more RESTful, you could consider following the HATEOAS principle (i.e. by using JSON-LD). |
Getting rid of the data object is reasonable, however as an api user how would you distinguish between the different validation errors? Most of the status codes are transport related. Current solution is to only add an error object for http 400 |
As for hateoas: I think this is not needed for a simple sync api and would require me to implement a lot more routes than actually needed. If we later on discover that we need it, we can always build on the current proposal |
Which kind of validation do you mean? |
Did you take a look at the feed creation route? There are multiple things that can go wrong: website does not exist, http basic auth does not work, URL does not exist etc. An error message is fine but sometimes you want to match an error code to show a custom translated message (eg client has a different locale than the server) PS: 415 is basically about content payload not being able to be handled and not validation, 422 is better but part of WebDAV and would also be used in more than one error case. What I'm trying to say is that you can't use http status codes for everything |
Another use case for error codes would be to show the user and password form again if feed creation failed due to authentication issues. |
@GisoBartels removed the data object nesting from the spec |
I didn't mean to use a different HTTP status code for each error or to omit the additional error code. That's fine. |
What do you think about the second wrapper object? |
Both are specified like you mentioned already ;) Yes wrapping the stuff in a folder property is the way to go since it's extensible |
I've added additional docs on:
|
Moved the API docs into a PR so it can be discussed more easily #1000 |
In addition to the old APIs I'd like to modernize the API since it seems more complicated than it needs to be and a lot of REST things are used in a wrong way
The main goals are the following
Move the shitty version in the URL from vX-X to a semantically versioned URL, e.g. /api/2.0.0/ or remove the version from the URL entirely by sending an HTTP request header (News-Api-Version: 2.0.0)
Use HTTP cache headers instead of requiring you to pass a last modified date
Crack down on the number of URLs, e.g. marking an item as read and starred would become
Actions on multiple resources should be handled at the correct endpoint, e.g. marking multiple items as read and starred would look like this:
Get rid of the id vs feedid + guidhash difference. I thought this was useful once, but we did not need it
Also implement a modified date for feeds and folders so you don't have to fetch everything all over again
Provide helper routes to make syncing a breeze. You normally have three usecases, those should be reflected by three routes
The fingerprint is a hash over the title, url, content and enclosures and if it is the same in the current database, only the status (read/starred) will be returned to the client.
The response will be the changes.
Furthermore creating feeds/folders that already exist should respond with the existing object instead of just returning an error message. This makes it very easy to create folders: you just submit the contents and if you get a 409 you just use the returned contents.
Ideas? Suggestions?
@David-Development @phedlund @icewind1991 @jangernert @cosenal
The text was updated successfully, but these errors were encountered: