Comb-through A.K.A. v2

[dscottboggs]

It has come to my attention that the Mastodon API has been updated a lot since the original work this was based on, and we need to comb through the API documentation ensuring that each type is implemented according to the spec.

When this is released, we will cut a new v2.0 release. Breaking changes will be a part of this.

Tasks for this PR

Comb through entities

• Account (122d9a2, Comb entities: Account #55)
• Admin::Account (72a2d86, Comb entities: admin #64)
• Admin::CanonicalEmailBlock (3a46fb7, Comb entities: admin #64)
• Admin::Cohort (d8ff573, Comb entities: admin #64)
• Admin::Dimension (c045a2a, Comb entities: admin #64)
• Admin::DomainAllow (c5e9171, f5d466b, Comb entities: admin #64)
• Admin::DomainBlock (e881c94, f5d466b, Comb entities: admin #64)
• Admin::EmailDomainBlock (d8d340c, f5d466b, Comb entities: admin #64)
• Admin::Ip (72a2d86, Comb entities: admin #64)
• Admin::IpBlock (4b5ddb1, Comb entities: admin #64)
• Admin::Measure (7285fac, Comb entities: admin #64)
• Admin::Report (edb9f32, Comb entities: admin #64)
• Announcement (f944ecc, Comb entities: announcement #66)
• Application (9a27c5a, Comb entities: Application #67)
• Context
• Conversation
• CustomEmoji (122d9a2 Comb entities: Account #55)
• DomainBlock (4fa96fe, Comb entities: instance #69)
• Error
• ExtendedDescription (aac9803, Comb entities: Small fries #79)
• FamiliarFollowers (7d0dde4, 37ff61c, Comb entities: Small fries #79)
• FeaturedTag (8728e37, Comb entities: Small fries #79)
• Filter (6ee2f27, abafaf2)
• FilterKeyword
• FilterResult
• FilterStatus
• IdentityProof already deprecated upstream. no sense in adding an entity just to deprecate it.
• Instance (4fa96fe, Comb entities: instance #69)
• List (725d660, Comb entities: Small fries #79)
• Marker (85f3e1d, Comb entities: Small fries #79)
• MediaAttachment (c39d8cd, Comb entities: media attachment #70)
• Notification (33933c7, Comb entities: notifications #71)
• Poll (aef7330, Comb entities: the ones that start with P #72)
• Preferences (6b56fdc, e79b859, Comb entities: the ones that start with P #72)
• PreviewCard (7bb26b4, Comb entities: the ones that start with P #72)
• Trends::Link (0e07453, Comb entities: the ones that start with P #72)
• Reaction (dde372a, Comb entities: Small fries #79)
• Relationship (66283d1, Comb entities: Small fries #79)
• Report (a1cdaa1, Comb entities: Small fries #79)
• Role (abafaf2 Comb entities: Account #55)
• Rule (4fa96fe, Comb entities: instance #69)
• ScheduledStatus (e824f24, Comb entities: status related #78)
• Search (46f80d1, Comb entities: Small fries #79)
• Status (3e60526, Comb entities: status related #78)
• StatusEdit (e453e1d, Comb entities: status related #78)
• StatusSource (67f131a, Comb entities: status related #78)
• Suggestion (239b5dc, Comb entities: Small fries #79)
• Tag (79de591, 52a0109, Comb entities: Small fries #79)
• Token (e6a7dfc, Comb entities: Small fries #79)
• V1::Filter (abafaf2)
• V1::Instance (4fa96fe, Comb entities: instance #69)
• WebPushSubscription (6287317, Comb entities: Small fries #79)

Entities, second pass

• Enums renamed for serde (Comb entities, second pass: serde rename #82)
• Enums impl IsVariant
• Examples tested (Comb entities/second pass/example tests #80)
• cargo doc (fix doc links #83)
• prelude (Update entities prelude #84)

Comb through methods

• apps (Comb methods: apps #91)
  • oauth (Comb methods: Oauth 2 #121 and Add OAuth revocation method #123)
  • emails
• accounts (Comb methods: Accounts #124)
  • bookmarks
  • favourites
  • mutes
  • blocks
  • domain_blocks
  • filters Comb methods: Filters #144
  • reports
  • follow_requests
  • endorsements
  • featured_tags
  • preferences
  • followed_tags
  • suggestions
  • tags
• statuses
  • media
  • polls
  • scheduled_statuses
• timelines
  • conversations
  • lists
  • markers
  • streaming
• notifications
  • push
• search
• instance
  • trends
  • directory
  • custom_emojis
  • announcements
• admin
  • admin/accounts
  • admin/domain_blocks
  • admin/reports
  • admin/trends
  • canonical_email_blocks
  • dimensions
  • domain_allows
  • email_domain_blocks
  • ip_blocks
  • measures
  • retention
• proofs
• oembed

Finishing up/other tasks

• Ensure mastodon_async::requests module is migrated entirely into mastodon_async_entities::forms
• Revisit/review the prelude and public-facing API.

[baummarten]

What do you prefer to do do with URLs that accept multiple optional parameters (like /api/v1/timelines/public), just add all of them with an Option type, or maybe some kind of a builder pattern?

[dscottboggs]

The previous authors seemed to be fans of using the builder pattern to generate structures which use serde of serialization into a urlencoded string, and I think it fits nicely. There are a couple changes I'm making to the way they did that as a part of v2:

1. I'd like to use the derive_builder crate
2. (and I wouldn't mind some criticism of this point) I'd like to include default values rather than use option types where it makes sense. For example, the public timelines type would be like

   #[derive(Builder, Default, Serialize, Deserialize, Clone)]
   pub struct TimelineQuery {
       #[builder(default)]
       pub local: bool,
       #[builder(default)]
       pub remote: bool,
       #[builder(default)]
       pub only_media: bool,
       #[builder(setter(into, strip_option))]
       pub max_id: Option<String>,
       #[builder(setter(into, strip_option))]
       pub since_id: Option<String>,
       #[builder(setter(into, strip_option))]
       pub min_id: Option<String>,
       #[builder(setter(into, strip_option))]
       pub limit: u8,
   }

   rather than

   #[derive(Builder, Default, Serialize, Deserialize, Clone)]
   pub struct TimelineQuery {
       #[builder(setter(strip_option))]
       pub local: Option<bool>,
       #[builder(setter(strip_option))]
       pub remote: Option<bool>,
       #[builder(setter(strip_option))]
       pub only_media: Option<bool>,
       #[builder(setter(into, strip_option))]
       pub max_id: Option<String>,
       #[builder(setter(into, strip_option))]
       pub since_id: Option<String>,
       #[builder(setter(into, strip_option))]
       pub min_id: Option<String>,
       #[builder(setter(into, strip_option))]
       pub limit: Option<u8>,
   }

   The idea is basically that unnecessary options are un-ergonomic, so anywhere where a default is specified and unlikely to change, make it the default here.
3. All fields should be public. These types may be useful to consumers of the library in other ways, and this also allows constructing the types without using the builder pattern if someone finds it inconvenient.

---

Another thought I have on this is that perhaps it would be a good idea to take a look at this section of the paged_routes! macro definition and its corresponding invokation to see if we can draw inspiration for making the way we do this more DRY.

Any thoughts?

[baummarten]

(1) Sure.
(2) If there's an obvious default then yeah, no need for an Option. The only disadvantage I see that there's no way to leave it unset, but since the default for booleans are given in the API description (and Eugen has gone on record that existing API won't be messed with), then the logic could be "if value=default then don't set anything", if that ever matters.
(3) That also makes sense, I don't think types for API queries could possibly have any invariants that need to be protected.

[VyrCossont]

Any way I can help out here? Is the comb branch where all of this work exists currently?

Would love to have this; the admin API will be critical for building automod tools for Mastodon, and I ended up implementing most of it in https://github.com/VyrCossont/mastodon-async/tree/admin-api before noticing this issue.

[dscottboggs]

Hello, yes, I am coming back to this soon but a couple of more urgent projects have come up, I should be wrapping them up soon (at least the urgent part). I really appreciate your work on the admin API, however, as you noted there may be some things which are already done. By all means, create a pull request (targeting the comb branch) and let's take a look at what needs done to get that merged.

[VyrCossont]

Right on. I'll take a look at the comb branch and see if anything from my admin-api branch is reusable or if there's other work I can easily pick up.

[VyrCossont]

Also, my take on builders and defaults is that, while unnecessary options are un-ergonomic:

1. #[builder(setter(strip_option))] makes this a non-issue from the builder's caller's perspective.
2. I'd rather not send parameters that aren't explicitly set. This makes the requests smaller and easier to read when debugging.
3. We're going to be dealing with at least 4 families of implementation of the Mastodon API: Mastodon/Glitch/Hometown, Pleroma/Akkoma, GotoSocial, and now Calckey (a Misskey fork), plus there's a pretty broad version spread of deployed instances of all of these. I think we should be conservative in what we send, in case one of them chokes on a parameter that it hasn't implemented yet.

[VyrCossont]

I tried implementing a builder with some mandatory and some optional parameters using derive_builder and serde_with:

use derive_builder::Builder;
use mastodon_async_entities::{report::Category, AccountId, RuleId, StatusId};
use serde_with::skip_serializing_none;

/// Form used to create a report
///
/// // Example
///
/// ```
/// use mastodon_async::{entities::{AccountId, report::Category}, requests::AddReportRequest};
/// let request = AddReportRequest::builder(AccountId::new("666")).category(Category::Spam).build();
/// ```
#[skip_serializing_none]
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Builder)]
#[builder(
    custom_constructor,
    build_fn(private, name = "try_build"),
    setter(into, strip_option)
)]
pub struct AddReportRequest {
    /// The account being reported.
    #[builder(private)]
    pub account_id: AccountId,
    /// Attach statuses to the report to provide additional context.
    #[builder(default)]
    pub status_ids: Option<Vec<StatusId>>,
    /// The reason for the report.
    #[builder(default)]
    pub comment: Option<String>,
    /// If the account is remote, should the report be forwarded to the remote admin?
    #[builder(default)]
    pub forward: Option<bool>,
    /// Machine-readable category of the report.
    #[builder(default)]
    pub category: Option<Category>,
    /// Rules broken by a [`Category::Violation`] report.
    #[builder(default)]
    pub rule_ids: Option<Vec<RuleId>>,
}

impl AddReportRequest {
    /// Start building a form for creating a report.
    pub fn builder(account_id: AccountId) -> AddReportRequestBuilder {
        let mut builder = AddReportRequestBuilder::create_empty();
        builder.account_id(account_id);
        builder
    }
}

impl AddReportRequestBuilder {
    /// Build the form for creating a report.
    pub fn build(&self) -> AddReportRequest {
        self.try_build()
            .expect("One or more required fields are missing!")
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use serde_json;

    #[test]
    fn test_serialize_request() {
        let request = AddReportRequest::builder(AccountId::new("666"))
            .category(Category::Spam)
            .build();
        let ser = serde_json::to_string(&request).expect("Couldn't serialize");
        assert_eq!(ser, r#"{"account_id":"666","category":"spam"}"#);
    }
}

This gives the caller two options, both safe: they can create an AddReportRequest directly and the type system forces them to provide mandatory params (as well as None or Default::default() for each optional param), or they can call AddReportRequest::builder(account_id), which takes the mandatory params, call builder methods for any optional params, and then .build().

The boilerplate cost of this is a #[builder(default)] annotation on every optional param and a #[builder(private)] annotation on every mandatory param (assuming we want to hide methods that are redundant with the ::builder(…) method that takes mandatory params), plus the struct-level #[skip_serializing_none] and #[builder(…)] annotations and the ::builder(…) and .build() methods. We can probably improve on this, but it doesn't seem horrible as is, especially compared to how many lines we can drop that are redundant handwritten builder methods and tests for errors that won't be able to happen any more.

[dscottboggs]

Looks good to me.

[dscottboggs]

@VyrCossont you may want to review the changes in this commit 035637f

[VyrCossont]

Filled in the reporting and admin APIs here: https://github.com/VyrCossont/mastodon-async/tree/comb

The above builder pattern is pretty much fine, I think. I'm going to convert the older builders to that style where it makes sense.

[dscottboggs]

This is great! Would you mind creating a PR for this?

[VyrCossont]

Sure, I'll open a draft and let you know once it's ready for review.

[VyrCossont]

Draft in #93. I'm concurrently working on an automod bot that uses mastodon-async, so the admin APIs I've implemented should get a workout, as well as the status creation API.