Fixed pagination regression

[muesli]

Pagination requests have been broken since b8bb5ae68. Only one of the parameters max_id, since_id or min_id must be supplied.

While this is something that could be documented and handled by the API user, this is a regression that breaks the behavior of existing projects and examples.

[mattn]

Test is failing.

[coveralls]

Coverage decreased (-0.02%) to 90.504% when pulling acbaf58d8ae63001b067b322cbd89ff4e0bc0828 on muesli:pagination-fix into 8f6192e26b66e06c5eea3a8d17e342ea57c4a86e on mattn:master.

[muesli]

Adjusted the tests to look like before the regression. Obviously the behavior change was also reflected there. Sorry I missed it, not sure how I managed to ignore the failed tests in my IDE, but I guess I was just focused on the changes in the commit that caused the regression and didn't notice there were later related commits to it.

[muesli]

@mattn nudge :smile:

[mattn]

@178inaba Sorry, I'm not familier to mastdon APIs recently. Could you please review this?

[muesli]

@mattn This isn't a change of Mastodon's API, this is merely a regression of go-mastodon's behavior compared to earlier releases:

re-using the Pagination response as supplied by the API would cause an invalid request, as multiple filters (Min, Max, Since) are supplied. We used to prefer Max over Since, but that changed in b8bb5ae when Min was added. This restores the old behavior while also supporting the new Min filter.

[muesli]

@178inaba @mattn Anything I can do to advance this? I feel like it's a fairly straight-forward fix for a regression in go-mastodon.

If you're not fine with merging this, that's alright, but please let me know because it currently breaks a couple of my projects (https://github.com/muesli/statootstics as an example), and I'd then go a head and work around it client-side if I have to.

Thank you!

[178inaba]

@mattn @muesli I missed this pull request notification. I'm sorry🙇‍♂️ I will check!

[muesli]

@178inaba Don't worry, looking forward to your review!

[178inaba]

@muesli Sorry for the late review. My review is that this pull request can not be merged.

I will write my recognition below.

Certainly, the past SinceID and MaxID were mutually exclusive. But there have been use cases where you have to specify both. That is #89. If you merge this pull request, it won't meet #89. I think there is no way to meet the #89 use case and maintain backward compatibility. (Please let me know if there is a good way!)

The pagination sample would need to be fixed as follows:

c := mastodon.NewClient(&mastodon.Config{
    Server:       "https://mstdn.jp",
    ClientID:     "client-id",
    ClientSecret: "client-secret",
})
var followers []*mastodon.Account
var pg mastodon.Pagination
for {
    fs, err := c.GetAccountFollowers(context.Background(), "1", &pg)
    if err != nil {
        log.Fatal(err)
    }
    followers = append(followers, fs...)
    if pg.MaxID == "" {
        break
    }
+   pg.SinceID = "0"
    time.Sleep(10 * time.Second)
}
for _, f := range followers {
    fmt.Println(f.Acct)
}

[muesli]

@178inaba Thanks for the detailed responses! Just one remark, the actual fix needs to read something like that:

...
pg.SinceID = ""
pg.MinID = ""
...