LemmyNet / lemmy

🐀 A link aggregator and forum for the fediverse
https://join-lemmy.org
GNU Affero General Public License v3.0
13.3k stars 882 forks source link

Reorganize api endpoints (fixes #2022) #5216

Open Nutomic opened 1 week ago

Nutomic commented 1 week ago

These changes are mainly to discuss what the new API v4 routes should look like. Once we come to an agreement, I will adjust it so that the existing API v3 remains usable with the same paths.

Nutomic commented 1 week ago

Also moved SiteView.my_user to a separate endpoint (/api/v3/account/my_user) and removed SiteView.taglines/custom_emojis which were already deprecated.

SleeplessOne1917 commented 6 days ago

Beyond the bit of reorganizing done here, I think it would be good to handle fetching individual resources by including the id in the route instead of the POST body, e.g. fetching a single comment would be /comments/<id> instead of /comment with an id in the body, and fetching multiple comments would just be GET /comments.

Besides that, I believe it could be worth looking into making some datatypes specific for returning as responses. We already have a few types like that, but many of them return views (e.g. CommentView, PostView, etc.) that have a lot of data that's not needed in the context of the part of a client that's making an API call, making the JSON response bigger than it needs to be.

There's probably more to be said, but these two points are the biggest on my mind so far.

dessalines commented 6 days ago

All the params for GET requests are currently like:

It'd probably require a bunch of strange manipulations to parse the required params into a distinct part of the route like /ID/ rather than just doing them uniformly like we're currently doing.

Besides that, I believe it could be worth looking into making some datatypes specific for returning as responses. We already have a few types like that, but many of them return views (e.g. CommentView, PostView, etc.) that have a lot of data that's not needed in the context of the part of a client that's making an API call, making the JSON response bigger than it needs to be.

I'd be perfectly good with creating slimmer versions of CommentView and PostView, that are used in different places, as long as they are predictably and strongly-typed with different names, and we're sure that these slimmer versions aren't going to be missing anything that clients need. Feel free to open an issue for that, as well as the redundant / unecessary columns in certain places.

SleeplessOne1917 commented 6 days ago

It'd probably require a bunch of strange manipulations to parse the required params into a distinct part of the route like /ID/ rather than just doing them uniformly like we're currently doing.

actix extractors make that pretty straightforward: there is an extractor for that that works basically the same as the query and body extractors Lemmy already uses.

Besides that, I'll gladly pour some time into tweaking the API design. It's something I've been interested in for awhile. Sometime in the next month I was thinking of opening a PR similar to this with some annotations as to why I made the choices I did.

Nutomic commented 3 days ago

Im not sure its worth changing the way request parameters are passed, because that would require a lot of work for client devs to make things compatible, for little or no benefit. With this PR all thats needed is changing a few paths while parameters are the same. Also there are cases like GetPost where the same item can be fetched in different ways.

But in general, this PR is only one part of the changes for API v4. If there are other changes you want to include, make a PR so we can discuss it (best change only a small part as example first, and then make full changes later once we agree that it makes sense).

Nutomic commented 2 days ago

Updated with routes for api v3 and api v4. For api v3 I added the my_user field back to /api/v3/site for backwards compat.

On the other hand I removed endpoints for unreleased features from api v3, so that client devs have an incentive to upgrade. This includes the new endpoints for taglines and emojis which were removed from /api/v3/site, so it may be necessary to add that back in.

Edit: Strangely the api tests fail with dependency 0.20.0-api-v4.0, but pass fine with file:../../lemmy-js-client/. Both have the exact same code. Any idea what might cause this?

dessalines commented 1 day ago

It must be failing at register user for some reason. The file... is never reliable in my experience, I've only ever gotten it to work with yalc when testing, not pnpm link

Also I'm seeing a few v3 references still:

rg v3 crates/ api_tests/

Its probably fine to keep the v3 API routes, as long as we say that we will not support them in the future.