Skip to content

Conversation

crazytonyli
Copy link
Contributor

@crazytonyli crazytonyli commented Apr 17, 2024

This PR supersedes #62 with a full support of accessing REST APIs with different context.

API design

All REST endpoints can be called using an api: WordPressAPI instance like this:

// Use different context to get different objects
api.users.forViewing.get(id: 1)
api.posts.forEditing.get(id: 1)
api.posts.forEmbedding.get(id: 1)

// Update uses "edit" context internally—not visible on the syntax level.
api.posts.update(id: 1, params: ...)

// Compiler error.
api.posts.get(id: 1)

Implementation

The above API is supported by a few namespace types.

classDiagram
    class Namespace["Namespace (protocol)"]
    class ContextualNamespace["ContextualNamespace (protocol)"]
    class AnyNamespace
    class EditNamespace
    class ViewNamespace
    class EmbedNamespace
    Namespace <|-- AnyNamespace
    Namespace <|-- ContextualNamespace
    ContextualNamespace <|-- EditNamespace
    ContextualNamespace <|-- ViewNamespace
    ContextualNamespace <|-- EmbedNamespace
Loading

When you call .[users|posts|...], you get an AnyNamespace<T> instance. If the REST resource model conforms to Contextual protocol, you can get those forViewing, forEditing, forEmbedding functions which returns special namespace accordingly.

At the moment all "contextual namespaces" can call get(id:) and list() functions. We can add more next.

Some REST endpoints have more functions other than CRUD, like get current user. For those, we'll need to manually implement them under appropriate namespace types. For example:

extension ViewNamespace where T == SparseUser {
public func getCurrent() async throws -> T.View {
let request = self.api.helper.retrieveCurrentUserRequest(context: .view)
let response = try await api.perform(request: request)
return try parseRetrieveUserResponseWithViewContext(response: response)
}
}
extension EditNamespace where T == SparseUser {
public func getCurrent() async throws -> T.Edit {
let request = self.api.helper.retrieveCurrentUserRequest(context: .edit)
let response = try await api.perform(request: request)
return try parseRetrieveUserResponseWithEditContext(response: response)
}
}
extension EmbedNamespace where T == SparseUser {
public func getCurrent() async throws -> T.Embed {
let request = self.api.helper.retrieveCurrentUserRequest(context: .embed)
let response = try await api.perform(request: request)
return try parseRetrieveUserResponseWithEmbedContext(response: response)
}
}

Support a new type

When adding a new type to the library, we'll need to manual write two things to hook them up with the above API design (see this posts example).

  1. Conforms to Contextual, whose implementation calls rust functions.
  2. Implement a .[users|posts] equivlant for the new type.

Next steps

Currently only get(id:) and list() functions are shared among all REST models. We can add more, like create(params:), update(id:params:), and delete(id:). Update: This is partly addressed in this PR: b935935

When adding functions to namespace types, i.e. implementing getCurrent under all three context. It's not ideal that we have to repeat the same implementation three times. It'd be good to further extract to reduce boilerplate code. Update: This is now address in this PR: f96f8fc.

}
}
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are made so that the Example app can work. I believe we want to remove these "legacy" API call functions, which will be done in separate PRs.

@oguzkocer
Copy link
Contributor

I just skimmed through the changes for now and noticed that this PR implements quite a bit of stuff for /posts. Unfortunately that goes against what we are trying to do with 0.1. As documented in #21, we actually want to remove the /posts implementation if we can - although I am not as sure about that because I know we are using it quite a bit on the Swift side.

@crazytonyli Any chance this implementation could be based on /users instead? I'd really like for all 3 platforms to be going in the same direction and I don't think we'll be working on /posts for a while, so I don't want that to be holding us back. I am also worried about the mix of polished & unpolished code, so I'd prefer if we didn't add to it.

If that's not possible, I have some ideas about how to at least contain it. So, let me know what you think!

@crazytonyli
Copy link
Contributor Author

@oguzkocer This PR implements an API design that can be automatically added to all REST models. In my chat with @jkmassel, he wanted to see what's required to give a new model the same API design implemented in this PR. Since we have already a post implementation in the rust library, I used that as an example (see the "Support a new type" section in the PR description).

I'm aware that we don't want to ship posts in 0.1. I'm okay with either reverting the posts changes in this PR or deleting the posts code entirely after this PR is reviewed and merged. The posts code in this PR is for demonstration after all.

@crazytonyli crazytonyli enabled auto-merge (squash) May 22, 2024 20:57
@crazytonyli crazytonyli merged commit a70ddcd into trunk May 22, 2024
@crazytonyli crazytonyli deleted the swift-package-users-contexts-shared-reading-api branch May 22, 2024 21:00
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.

3 participants