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

Support /api/v1/instance and /api/v2/instance #3153

Closed
wants to merge 2 commits into from
Closed

Support /api/v1/instance and /api/v2/instance #3153

wants to merge 2 commits into from

Conversation

nikclayton
Copy link
Contributor

Start to split the API code in to v1 and v2 interfaces.

Migrate the getInstance() function (rename to instance() for consistency with most other API functions) to v1, and create a v2 equivalent. At the moment they're identical because they return the same type.

Modify InstanceInfoRepository to prefer the v2 call, falling back to v1 if that fails (then the cache, or defaults, per previous behaviour).

Create factory methods for InstanceInfo and InstanceEntity to keep related code together, ditto for defaults.

Update consumers and tests.

Fixes #3146

Start to split the API code in to v1 and v2 interfaces.

Migrate the getInstance() function (rename to instance() for consistency with
most other API functions) to v1, and create a v2 equivalent. At the moment
they're identical because they return the same type.

Modify InstanceInfoRepository to prefer the v2 call, falling back to v1 if
that fails (then the cache, or defaults, per previous behaviour).

Create factory methods for InstanceInfo and InstanceEntity to keep related
code together, ditto for defaults.

Update consumers and tests.

Fixes #3146
@nikclayton
Copy link
Contributor Author

Keeping this as a draft, to highlight and discuss a couple of points.

First, obviously having MastodonApi, MastodonApiV1, and MastodonApiV2 is a bad idea. For ease of code review I didn't want to migrate everything from MastodonApi in to one of the other two until there's a chance to discuss this. So this just deals with the instance() method at the moment.

Second, I'm not convinced about the package location.

Instead of network, I'm thinking there should be api.v1 and api.v2 packages. These packages would contain both the API definitions, and also the classes that encode the bodies of requests and responses.

The instance() method was straightforward, as both the v1 and v2 APIs (https://docs.joinmastodon.org/methods/instance/) return a V1::Instance (https://docs.joinmastodon.org/entities/Instance/), so I didn't need to create a different response type.

But e.g,. the filters API call returns different types for v1 and v2 calls, so they'll need to be kept separate.

So perhaps packages:

  • api.v1.request
  • api.v1.response
  • api.v2.request
  • api.v2.response

would make sense, and move everything under there?

That could be done as a big-bang migration, or on a call-by-call basis (multiple PRs, each PR dealing with a different API call).

@nikclayton nikclayton closed this by deleting the head repository Aug 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prefer /api/v2/instance
1 participant