From 78d8d7fc430a59c004c4408ad498ea5eae2d7b53 Mon Sep 17 00:00:00 2001 From: rjp Date: Tue, 23 May 2023 08:27:55 +0100 Subject: [PATCH 1/6] Keep the API JSON around (briefly) For my archival purposes, I like to keep the raw API JSON around as it often contains things not picked up by the various objects. e.g. getting a status from my Akkoma instance contains a whole `pleroma` extension block that's not reflected in `mastodon.Status`. Maybe this should be gated by an option when creating the `Client` since it's probably only relevant to 1% of library users. --- mastodon.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/mastodon.go b/mastodon.go index c704e10..ff58f28 100644 --- a/mastodon.go +++ b/mastodon.go @@ -29,6 +29,7 @@ type Client struct { http.Client Config *Config UserAgent string + LastJSON []byte } func (c *Client) doAPI(ctx context.Context, method string, uri string, params interface{}, res interface{}, pg *Pagination) error { @@ -125,7 +126,18 @@ func (c *Client) doAPI(ctx context.Context, method string, uri string, params in *pg = *pg2 } } - return json.NewDecoder(resp.Body).Decode(&res) + + // If we want to store the JSON received, we absolutely have to + // read all of it. But we restrict ourselves to a max of 100M. + safer := &io.LimitedReader{resp.Body, 100 * 1_048_576} + c.LastJSON, err = io.ReadAll(safer) + + if err != nil || c.LastJSON == nil { + return err + } + + // ...which means we can't use `NewDecoder.Decode` any more. + return json.Unmarshal(c.LastJSON, &res) } // NewClient returns a new mastodon API client. From 63cf193a13af1c9304f57a4956029fe6a91ef821 Mon Sep 17 00:00:00 2001 From: rjp Date: Tue, 23 May 2023 08:33:15 +0100 Subject: [PATCH 2/6] Gate the JSON saving behind a client flag This way the library will behave as it used to for anyone who doesn't explicitly set `client.SaveJSON` to be `true`. No surprises = good. --- mastodon.go | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/mastodon.go b/mastodon.go index ff58f28..2bd61c3 100644 --- a/mastodon.go +++ b/mastodon.go @@ -29,6 +29,7 @@ type Client struct { http.Client Config *Config UserAgent string + SaveJSON bool LastJSON []byte } @@ -127,17 +128,22 @@ func (c *Client) doAPI(ctx context.Context, method string, uri string, params in } } - // If we want to store the JSON received, we absolutely have to - // read all of it. But we restrict ourselves to a max of 100M. - safer := &io.LimitedReader{resp.Body, 100 * 1_048_576} - c.LastJSON, err = io.ReadAll(safer) + if c.SaveJSON { + // We want to store the JSON received -> we absolutely have to + // read all of it. But we restrict ourselves to a max of 100M. + safer := &io.LimitedReader{resp.Body, 100 * 1_048_576} + c.LastJSON, err = io.ReadAll(safer) - if err != nil || c.LastJSON == nil { - return err + if err != nil || c.LastJSON == nil { + return err + } + + // ...which means we can't use `NewDecoder.Decode` any more. + return json.Unmarshal(c.LastJSON, &res) + } else { + // We don't want the JSON, just do the previous streaming decode. + return json.NewDecoder(resp.Body).Decode(&res) } - - // ...which means we can't use `NewDecoder.Decode` any more. - return json.Unmarshal(c.LastJSON, &res) } // NewClient returns a new mastodon API client. From 333d71452a43e5d847ac66da0d5024d1e94e903b Mon Sep 17 00:00:00 2001 From: rjp Date: Tue, 23 May 2023 09:00:54 +0100 Subject: [PATCH 3/6] A simple test for retained JSON --- status_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/status_test.go b/status_test.go index 2cac4b8..b3f5e8c 100644 --- a/status_test.go +++ b/status_test.go @@ -7,6 +7,7 @@ import ( "net/http" "net/http/httptest" "os" + "strings" "testing" ) @@ -37,6 +38,45 @@ func TestGetFavourites(t *testing.T) { } } +func TestGetFavouritesSavedJSON(t *testing.T) { + ourJSON := `[{"content": "foo"}, {"content": "bar"}]` + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, ourJSON) + })) + defer ts.Close() + + client := NewClient(&Config{ + Server: ts.URL, + ClientID: "foo", + ClientSecret: "bar", + AccessToken: "zoo", + }) + + client.SaveJSON = true + + favs, err := client.GetFavourites(context.Background(), nil) + if err != nil { + t.Fatalf("should not be fail: %v", err) + } + if len(favs) != 2 { + t.Fatalf("result should be two: %d", len(favs)) + } + if favs[0].Content != "foo" { + t.Fatalf("want %q but %q", "foo", favs[0].Content) + } + if favs[1].Content != "bar" { + t.Fatalf("want %q but %q", "bar", favs[1].Content) + } + + // We get a trailing `\n` from the API which we need to trim + // off before we compare it with our literal above. + theirJSON := strings.TrimSpace(string(client.LastJSON)) + + if theirJSON != ourJSON { + t.Fatalf("want %q but %q", ourJSON, theirJSON) + } +} + func TestGetBookmarks(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprintln(w, `[{"content": "foo"}, {"content": "bar"}]`) From 51e4324c7a00596a1ab0d62a22688d204384c6f1 Mon Sep 17 00:00:00 2001 From: rjp Date: Thu, 25 May 2023 08:05:58 +0100 Subject: [PATCH 4/6] Better handling of "save the JSON" As suggested by 'mattn, use a `TeeReader` with an `io.Writer` to keep the semantics of `json.NewDecoder().Decode()` whilst also being able to save the JSON if requested by the client. However we need to use our own interface (`WriteResetter`) in order to be able to call `Reset()` on the underlying buffer (which does somewhat limit the usage to buffers instead of generic `io.Writer` and may not be the 100% ideal solution.) --- mastodon.go | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/mastodon.go b/mastodon.go index 2bd61c3..309bac2 100644 --- a/mastodon.go +++ b/mastodon.go @@ -24,13 +24,17 @@ type Config struct { AccessToken string } +type WriteResetter interface { + io.Writer + Reset() +} + // Client is a API client for mastodon. type Client struct { http.Client - Config *Config - UserAgent string - SaveJSON bool - LastJSON []byte + Config *Config + UserAgent string + JSONWriter WriteResetter } func (c *Client) doAPI(ctx context.Context, method string, uri string, params interface{}, res interface{}, pg *Pagination) error { @@ -128,20 +132,10 @@ func (c *Client) doAPI(ctx context.Context, method string, uri string, params in } } - if c.SaveJSON { - // We want to store the JSON received -> we absolutely have to - // read all of it. But we restrict ourselves to a max of 100M. - safer := &io.LimitedReader{resp.Body, 100 * 1_048_576} - c.LastJSON, err = io.ReadAll(safer) - - if err != nil || c.LastJSON == nil { - return err - } - - // ...which means we can't use `NewDecoder.Decode` any more. - return json.Unmarshal(c.LastJSON, &res) + if c.JSONWriter != nil { + c.JSONWriter.Reset() + return json.NewDecoder(io.TeeReader(resp.Body, c.JSONWriter)).Decode(&res) } else { - // We don't want the JSON, just do the previous streaming decode. return json.NewDecoder(resp.Body).Decode(&res) } } From 08be497fae653fed9d6c12f14e4ec215cfba7237 Mon Sep 17 00:00:00 2001 From: rjp Date: Thu, 25 May 2023 08:36:53 +0100 Subject: [PATCH 5/6] Revert to simple `io.Writer` with client-resetting After going through several other solutions, there's no clean way to use an `io.Writer` but also be able to call `Reset()` between writes. Filehandles can't `Reset()` and buffers can't `Seek()`. But then if you supplied `os.Stderr`, you'd be expecting appending and a `Seek()` would be nonsense. My own preference would be to make `JSONWriter` a strict `*bytes.Buffer` which lets the library handle the resetting and the client do any outputting if required - I can't see much value in ever supplying a non-`bytes.Buffer` as `JSONWriter`. --- mastodon.go | 8 +------- status_test.go | 6 ++++-- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/mastodon.go b/mastodon.go index 309bac2..385efcb 100644 --- a/mastodon.go +++ b/mastodon.go @@ -24,17 +24,12 @@ type Config struct { AccessToken string } -type WriteResetter interface { - io.Writer - Reset() -} - // Client is a API client for mastodon. type Client struct { http.Client Config *Config UserAgent string - JSONWriter WriteResetter + JSONWriter io.Writer } func (c *Client) doAPI(ctx context.Context, method string, uri string, params interface{}, res interface{}, pg *Pagination) error { @@ -133,7 +128,6 @@ func (c *Client) doAPI(ctx context.Context, method string, uri string, params in } if c.JSONWriter != nil { - c.JSONWriter.Reset() return json.NewDecoder(io.TeeReader(resp.Body, c.JSONWriter)).Decode(&res) } else { return json.NewDecoder(resp.Body).Decode(&res) diff --git a/status_test.go b/status_test.go index b3f5e8c..a54b931 100644 --- a/status_test.go +++ b/status_test.go @@ -1,6 +1,7 @@ package mastodon import ( + "bytes" "context" "fmt" "io/ioutil" @@ -52,7 +53,8 @@ func TestGetFavouritesSavedJSON(t *testing.T) { AccessToken: "zoo", }) - client.SaveJSON = true + var buf bytes.Buffer + client.JSONWriter = &buf favs, err := client.GetFavourites(context.Background(), nil) if err != nil { @@ -70,7 +72,7 @@ func TestGetFavouritesSavedJSON(t *testing.T) { // We get a trailing `\n` from the API which we need to trim // off before we compare it with our literal above. - theirJSON := strings.TrimSpace(string(client.LastJSON)) + theirJSON := strings.TrimSpace(string(buf.Bytes())) if theirJSON != ourJSON { t.Fatalf("want %q but %q", ourJSON, theirJSON) From 9921b587a70df53ca58215369675d41bdf5d251c Mon Sep 17 00:00:00 2001 From: rjp Date: Thu, 25 May 2023 11:09:41 +0100 Subject: [PATCH 6/6] Best solution that's type-safe but also extensible Supply an `io.Writer` but check it against the `WriterResetter` interface to see if we can call `Reset()`. Which is a neat solution! Add a new test to check that a supplied `bytes.Buffer` gets reset between API calls. --- mastodon.go | 8 +++++++ status_test.go | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/mastodon.go b/mastodon.go index 385efcb..fadf7c7 100644 --- a/mastodon.go +++ b/mastodon.go @@ -24,6 +24,11 @@ type Config struct { AccessToken string } +type WriterResetter interface { + io.Writer + Reset() +} + // Client is a API client for mastodon. type Client struct { http.Client @@ -128,6 +133,9 @@ func (c *Client) doAPI(ctx context.Context, method string, uri string, params in } if c.JSONWriter != nil { + if resetter, ok := c.JSONWriter.(WriterResetter); ok { + resetter.Reset() + } return json.NewDecoder(io.TeeReader(resp.Body, c.JSONWriter)).Decode(&res) } else { return json.NewDecoder(resp.Body).Decode(&res) diff --git a/status_test.go b/status_test.go index a54b931..c520b82 100644 --- a/status_test.go +++ b/status_test.go @@ -39,6 +39,70 @@ func TestGetFavourites(t *testing.T) { } } +func TestGetFavouritesSavedJSONTwice(t *testing.T) { + ourJSON := `[{"content": "foo"}, {"content": "bar"}]` + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, ourJSON) + })) + defer ts.Close() + + client := NewClient(&Config{ + Server: ts.URL, + ClientID: "foo", + ClientSecret: "bar", + AccessToken: "zoo", + }) + + var buf bytes.Buffer + client.JSONWriter = &buf + + favs, err := client.GetFavourites(context.Background(), nil) + if err != nil { + t.Fatalf("should not be fail: %v", err) + } + if len(favs) != 2 { + t.Fatalf("result should be two: %d", len(favs)) + } + if favs[0].Content != "foo" { + t.Fatalf("want %q but %q", "foo", favs[0].Content) + } + if favs[1].Content != "bar" { + t.Fatalf("want %q but %q", "bar", favs[1].Content) + } + + // We get a trailing `\n` from the API which we need to trim + // off before we compare it with our literal above. + theirJSON := strings.TrimSpace(string(buf.Bytes())) + + if theirJSON != ourJSON { + t.Fatalf("want %q but %q", ourJSON, theirJSON) + } + + // Now we call the API again to see if we get the same or doubled JSON. + + favs, err = client.GetFavourites(context.Background(), nil) + if err != nil { + t.Fatalf("should not be fail: %v", err) + } + if len(favs) != 2 { + t.Fatalf("result should be two: %d", len(favs)) + } + if favs[0].Content != "foo" { + t.Fatalf("want %q but %q", "foo", favs[0].Content) + } + if favs[1].Content != "bar" { + t.Fatalf("want %q but %q", "bar", favs[1].Content) + } + + // We get a trailing `\n` from the API which we need to trim + // off before we compare it with our literal above. + theirJSON = strings.TrimSpace(string(buf.Bytes())) + + if theirJSON != ourJSON { + t.Fatalf("want %q but %q", ourJSON, theirJSON) + } +} + func TestGetFavouritesSavedJSON(t *testing.T) { ourJSON := `[{"content": "foo"}, {"content": "bar"}]` ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {