Skip to content
This repository has been archived by the owner on Sep 8, 2021. It is now read-only.

API Migration #820

Open
heyarne opened this issue Oct 28, 2018 · 5 comments
Open

API Migration #820

heyarne opened this issue Oct 28, 2018 · 5 comments
Labels
in: rest Issues in the REST API. neverstale This label will be removed soon status: waiting-for-feedback The replication have not been confirmed yet. Or no implementer or no PRs or tips to solve yet. type: request There is no implementer. Or there is no support from the maintainer. Please implement it and appeal.

Comments

@heyarne
Copy link
Contributor

heyarne commented Oct 28, 2018

Problem description

It arose as a consequence of various issues that we will not be able to hold api compatibility with subsonic 100%. There were a couple of pull requests adressing this, see #553 and follow the links to know more.

The way I see it is this: The Subsonic API has a healthy ecosystem around it; it enables desktop, mobile and other web clients. At the same time, to be frank, it's a horrible API. It's practically unusable without a copy of the documentation open at all times and leads to insecurities (see #69 for example).

This issue is meant as a kind of meta issue. I know there have been discussions about this in various places but it makes sense to consolidate them and have one place to point to as guidance.

Suggested solution

It would be detrimental to airsonic and everyone using it to ditch the subsonic api for now (I don't think this has ever been discussed as a short-term goal anyways but it might make sense to state this explicitly). In order to balance airsonic's goals of being a community driven fork that's open towards new features, we should implement incompatible features against the new api. My approach for this would be this:

Introduce a new URL scheme. Keep /rest for now for compatibility. Forward /api/legacy to /rest. /api/v1 is the place for the new and shiny airsonic api.

We can start by…

a) …modernizing the existing endpoints by using proper HTTP verbs and HTTP Basic auth. This is done in #553 from what I can tell.
b) …implementing new endpoints. While this surely helps solving some of the open issues (see #776 for example which needs an endpoint to store the last.fm token), it overlaps with a) partially (no token storage without auth).

Any thoughts on this? I'm not 100% sure if this issue actually makes sense or if it's a step backwards discussing things that have already been discussed and decided on. I think however that if we have many heads deciding on things (e.g. the community) it makes sense to have one place where it's decided (e.g. this issue??) for the sake of manageability.

As an aside: What happened to Kotlin integration? I remember it popping up in a pull request but it's been put on hold? FWIW I'm a strong proponent of Kotlin; it means being able to leave code that's already written practically untouched and seamless interop is one of the explicitly stated goals of the design team behind it. Heck, you can even convert Java code to Kotlin code by hitting a keyboard combo in IntelliJ. I know this is mostly a personal opinion, but Kotlin is just waayyy nicer to write and read than Java code.

@heyarne
Copy link
Contributor Author

heyarne commented Oct 28, 2018

@jooola jooola added the type: enhancement-closed What was previously labeled enhancement. For archiving. Will be organized later. label Oct 28, 2018
@muff1nman
Copy link
Contributor

I believe what your describing wrt the api is pretty much what I started implementing in #553. I had put a hold on merging it for a couple reasons:

  • try to prioritize working on some important bugs
  • allow time for others to review
  • need to implement authentication/authorization scheme

I still would like to get it merged at some point because it would allow us a road to move on from the legacy api.

@jooola
Copy link
Contributor

jooola commented Oct 29, 2018

It's just a detail, but I think we should really consider using something like swagger documentation tools using annotations. This will help a lot for devs to move to the new API

@heyarne
Copy link
Contributor Author

heyarne commented Oct 30, 2018

@muff1nman I see, makes sense. I left a couple of comments in the pull request itself. May I ask what you mean by "implementing an authentication / authorization scheme" exactly? I saw in the pull request that grounds for HTTP Basic Auth have been laid out. OAuth might be interesting for sure, seeing how different user roles / permissions might be mapped to OAuth scopes, but I think picking the low-hanging fruit and starting with HTTP Basic Auth would already be a huge step forward, considering security and interoperability with other tooling just by adhering to standards.
Btw, I've also seen #474 (which I already commented on but forgot again...), which is related to this as well.

@jooola I think so, too. It will make the progress of documenting everything and keeping that documentation up-to-date less tedious.

@stale
Copy link

stale bot commented Apr 6, 2019

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale This label will be removed soon label Apr 6, 2019
@stale stale bot closed this as completed Oct 3, 2019
@muff1nman muff1nman reopened this Oct 6, 2019
@stale stale bot removed the stale This label will be removed soon label Oct 6, 2019
@jooola jooola added the neverstale This label will be removed soon label Oct 6, 2019
@eharris eharris added the in: rest Issues in the REST API. label Oct 9, 2019
@tesshucom tesshucom added type: request There is no implementer. Or there is no support from the maintainer. Please implement it and appeal. status: waiting-for-feedback The replication have not been confirmed yet. Or no implementer or no PRs or tips to solve yet. and removed type: enhancement-closed What was previously labeled enhancement. For archiving. Will be organized later. labels May 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
in: rest Issues in the REST API. neverstale This label will be removed soon status: waiting-for-feedback The replication have not been confirmed yet. Or no implementer or no PRs or tips to solve yet. type: request There is no implementer. Or there is no support from the maintainer. Please implement it and appeal.
Projects
None yet
Development

No branches or pull requests

5 participants