diff --git a/CHANGELOG.md b/CHANGELOG.md index 3888094e..832b793b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - Dev: Improve YouTube tests. (#284) - Dev: Resolver Check now returns a context. (#287) - Dev: Improve Wikipedia tests. (#286) +- Dev: Improve Imgur tests. (#289) ## 1.2.3 diff --git a/internal/resolvers/imgur/helpers.go b/internal/resolvers/imgur/helpers.go index ef462054..4f3ea613 100644 --- a/internal/resolvers/imgur/helpers.go +++ b/internal/resolvers/imgur/helpers.go @@ -2,7 +2,6 @@ package imgur import ( "bytes" - "fmt" "log" "net/http" "net/url" @@ -85,6 +84,7 @@ func finalizeMiniImage(mini *miniImage) { mini.Link = linkURL.String() } else { log.Println("[IMGUR] Error making smaller thumbnail for image:", err, mini) + mini.Link = "" } } } @@ -95,18 +95,11 @@ func finalizeMiniImage(mini *miniImage) { } } -func internalServerError(message string) (*resolver.Response, time.Duration, error) { - return &resolver.Response{ - Status: http.StatusInternalServerError, - Message: "imgur resolver error: " + resolver.CleanResponse(message), - }, cache.NoSpecialDur, nil -} - func buildTooltip(miniData miniImage) (*resolver.Response, time.Duration, error) { var tooltip bytes.Buffer if err := imageTooltipTemplate.Execute(&tooltip, &miniData); err != nil { - return internalServerError(fmt.Sprintf("Error building template: %s", err.Error())) + return resolver.Errorf("Imgur template error: %s", err) } return &resolver.Response{ diff --git a/internal/resolvers/imgur/helpers_test.go b/internal/resolvers/imgur/helpers_test.go deleted file mode 100644 index 86550f44..00000000 --- a/internal/resolvers/imgur/helpers_test.go +++ /dev/null @@ -1,79 +0,0 @@ -package imgur - -import ( - "net/http" - "testing" - "time" - - "github.com/Chatterino/api/pkg/cache" - "github.com/Chatterino/api/pkg/resolver" - qt "github.com/frankban/quicktest" -) - -func TestInternalServerRror(t *testing.T) { - c := qt.New(t) - - type tTest struct { - label string - input string - expectedResponse *resolver.Response - expectedDuration time.Duration - expectedError error - } - - tests := []tTest{ - { - label: "html", - input: "error message xD", - expectedResponse: &resolver.Response{ - Status: http.StatusInternalServerError, - Message: "imgur resolver error: error message <b>xD</b>", - }, - expectedDuration: cache.NoSpecialDur, - expectedError: nil, - }, - } - - for _, test := range tests { - c.Run(test.label, func(c *qt.C) { - response, duration, err := internalServerError(test.input) - c.Assert(response, qt.DeepEquals, test.expectedResponse) - c.Assert(duration, qt.Equals, test.expectedDuration) - c.Assert(err, qt.Equals, test.expectedError) - }) - } -} - -func TestBuildTooltip(t *testing.T) { - c := qt.New(t) - - type tTest struct { - label string - input miniImage - expectedResponse *resolver.Response - expectedDuration time.Duration - expectedError error - } - - tests := []tTest{ - { - label: "empty image", - input: miniImage{}, - expectedResponse: &resolver.Response{ - Status: http.StatusOK, - Tooltip: "%3Cdiv%20style=%22text-align:%20left%3B%22%3E%3Cli%3E%3Cb%3EUploaded:%3C%2Fb%3E%20%3C%2Fli%3E%3C%2Fdiv%3E", - }, - expectedDuration: cache.NoSpecialDur, - expectedError: nil, - }, - } - - for _, test := range tests { - c.Run(test.label, func(c *qt.C) { - response, duration, err := buildTooltip(test.input) - c.Assert(response, qt.DeepEquals, test.expectedResponse) - c.Assert(duration, qt.Equals, test.expectedDuration) - c.Assert(err, qt.Equals, test.expectedError) - }) - } -} diff --git a/internal/resolvers/imgur/initialize.go b/internal/resolvers/imgur/initialize.go index 614788dc..018cba27 100644 --- a/internal/resolvers/imgur/initialize.go +++ b/internal/resolvers/imgur/initialize.go @@ -8,6 +8,7 @@ import ( "github.com/Chatterino/api/internal/logger" "github.com/Chatterino/api/pkg/config" "github.com/Chatterino/api/pkg/resolver" + "github.com/koffeinsource/go-imgur" ) var ( @@ -24,5 +25,12 @@ func Initialize(ctx context.Context, cfg config.APIConfig, pool db.Pool, resolve return } - *resolvers = append(*resolvers, NewResolver(ctx, cfg, pool)) + imgurClient := &imgur.Client{ + HTTPClient: resolver.HTTPClient(), + Log: &NullLogger{}, + ImgurClientID: cfg.ImgurClientID, + RapidAPIKEY: "", + } + + *resolvers = append(*resolvers, NewResolver(ctx, cfg, pool, imgurClient)) } diff --git a/internal/resolvers/imgur/initialize_test.go b/internal/resolvers/imgur/initialize_test.go new file mode 100644 index 00000000..f2344878 --- /dev/null +++ b/internal/resolvers/imgur/initialize_test.go @@ -0,0 +1,38 @@ +package imgur + +import ( + "context" + "testing" + + "github.com/Chatterino/api/internal/logger" + "github.com/Chatterino/api/pkg/config" + "github.com/Chatterino/api/pkg/resolver" + qt "github.com/frankban/quicktest" + "github.com/pashagolub/pgxmock" +) + +func TestInitialize(t *testing.T) { + ctx := logger.OnContext(context.Background(), logger.NewTest()) + c := qt.New(t) + + pool, err := pgxmock.NewPool() + c.Assert(err, qt.IsNil) + + c.Run("No credentials", func(c *qt.C) { + cfg := config.APIConfig{} + customResolvers := []resolver.Resolver{} + c.Assert(customResolvers, qt.HasLen, 0) + Initialize(ctx, cfg, pool, &customResolvers) + c.Assert(customResolvers, qt.HasLen, 0) + }) + + c.Run("Credentials", func(c *qt.C) { + cfg := config.APIConfig{ + ImgurClientID: "test", + } + customResolvers := []resolver.Resolver{} + c.Assert(customResolvers, qt.HasLen, 0) + Initialize(ctx, cfg, pool, &customResolvers) + c.Assert(customResolvers, qt.HasLen, 1) + }) +} diff --git a/internal/resolvers/imgur/loader.go b/internal/resolvers/imgur/loader.go index a8891785..83a8ebe0 100644 --- a/internal/resolvers/imgur/loader.go +++ b/internal/resolvers/imgur/loader.go @@ -5,6 +5,7 @@ import ( "net/http" "time" + "github.com/Chatterino/api/internal/logger" "github.com/Chatterino/api/pkg/cache" "github.com/Chatterino/api/pkg/resolver" "github.com/Chatterino/api/pkg/utils" @@ -16,12 +17,23 @@ type Loader struct { } func (l *Loader) Load(ctx context.Context, urlString string, r *http.Request) (*resolver.Response, time.Duration, error) { + log := logger.FromContext(ctx) + genericInfo, _, err := l.apiClient.GetInfoFromURL(urlString) if err != nil { - return &resolver.Response{ - Status: http.StatusOK, - Tooltip: "Error getting imgur API information for URL", - }, cache.NoSpecialDur, resolver.ErrDontHandle + log.Warnw("Error getting imgur info from URL", + "url", urlString, + "error", err, + ) + return nil, cache.NoSpecialDur, resolver.ErrDontHandle + } + + if genericInfo == nil { + log.Warnw("Missing imgur info", + "url", urlString, + "error", err, + ) + return nil, cache.NoSpecialDur, resolver.ErrDontHandle } var miniData miniImage @@ -59,14 +71,17 @@ func (l *Loader) Load(ctx context.Context, urlString string, r *http.Request) (* miniData.Title = ptr.Title miniData.Description = ptr.Description } else { - return &resolver.Response{ - Status: http.StatusOK, - Tooltip: "Error getting imgur API information for URL", - }, cache.NoSpecialDur, nil + log.Warnw("Missing relevant imgur response", + "url", urlString, + ) + + return nil, resolver.NoSpecialDur, resolver.ErrDontHandle } // Proxy imgur thumbnails - miniData.Link = utils.FormatThumbnailURL(l.baseURL, r, miniData.Link) + if miniData.Link != "" { + miniData.Link = utils.FormatThumbnailURL(l.baseURL, r, miniData.Link) + } return buildTooltip(miniData) } diff --git a/internal/resolvers/imgur/resolver.go b/internal/resolvers/imgur/resolver.go index a2bd5185..a8da85e8 100644 --- a/internal/resolvers/imgur/resolver.go +++ b/internal/resolvers/imgur/resolver.go @@ -38,15 +38,10 @@ func (r *Resolver) Name() string { return "imgur" } -func NewResolver(ctx context.Context, cfg config.APIConfig, pool db.Pool) *Resolver { +func NewResolver(ctx context.Context, cfg config.APIConfig, pool db.Pool, imgurClient ImgurClient) *Resolver { loader := &Loader{ - baseURL: cfg.BaseURL, - apiClient: &imgur.Client{ - HTTPClient: resolver.HTTPClient(), - Log: &NullLogger{}, - ImgurClientID: cfg.ImgurClientID, - RapidAPIKEY: "", - }, + baseURL: cfg.BaseURL, + apiClient: imgurClient, } r := &Resolver{ diff --git a/internal/resolvers/imgur/resolver_test.go b/internal/resolvers/imgur/resolver_test.go index d3e360ac..05326beb 100644 --- a/internal/resolvers/imgur/resolver_test.go +++ b/internal/resolvers/imgur/resolver_test.go @@ -2,12 +2,22 @@ package imgur import ( "context" + "errors" + "net/http" "net/url" "testing" + "time" "github.com/Chatterino/api/internal/logger" + "github.com/Chatterino/api/internal/mocks" + "github.com/Chatterino/api/pkg/config" "github.com/Chatterino/api/pkg/resolver" + "github.com/Chatterino/api/pkg/utils" qt "github.com/frankban/quicktest" + "github.com/golang/mock/gomock" + "github.com/jackc/pgx/v4" + "github.com/koffeinsource/go-imgur" + "github.com/pashagolub/pgxmock" ) func testCheck(ctx context.Context, resolver resolver.Resolver, c *qt.C, urlString string) bool { @@ -48,3 +58,239 @@ func TestCheck(t *testing.T) { c.Assert(testCheck(ctx, resolver, c, u), qt.IsFalse) } } + +func TestResolver(t *testing.T) { + ctx := logger.OnContext(context.Background(), logger.NewTest()) + ctrl := gomock.NewController(t) + defer ctrl.Finish() + c := qt.New(t) + + imgurClient := mocks.NewMockImgurClient(ctrl) + + datetime := time.Date(2019, time.November, 10, 23, 0, 0, 0, time.UTC).Unix() + + pool, _ := pgxmock.NewPool() + + cfg := config.APIConfig{ + BaseURL: "https://example.com/", + } + + r := NewResolver(ctx, cfg, pool, imgurClient) + + c.Assert(r, qt.IsNotNil) + + c.Run("Name", func(c *qt.C) { + c.Assert(r.Name(), qt.Equals, "imgur") + }) + + c.Run("Check", func(c *qt.C) { + type checkTest struct { + label string + input *url.URL + expected bool + } + + tests := []checkTest{ + { + label: "Matching domain, no WWW", + input: utils.MustParseURL("https://imgur.com"), + expected: true, + }, + { + label: "Matching domain, WWW", + input: utils.MustParseURL("https://www.imgur.com"), + expected: true, + }, + { + label: "Matching subdomain", + input: utils.MustParseURL("https://m.imgur.com"), + expected: true, + }, + { + label: "Non-matching domain", + input: utils.MustParseURL("https://example.com/emotes/566ca04265dbbdab32ec054a"), + expected: false, + }, + } + + for _, test := range tests { + c.Run(test.label, func(c *qt.C) { + _, output := r.Check(ctx, test.input) + c.Assert(output, qt.Equals, test.expected) + }) + } + }) + + c.Run("Run", func(c *qt.C) { + c.Run("Dont handle", func(c *qt.C) { + type runTest struct { + label string + inputURL *url.URL + inputEmoteHash string + inputReq *http.Request + err error + info *imgur.GenericInfo + } + + tests := []runTest{ + { + label: "client error", + inputURL: utils.MustParseURL("https://betterttv.com/user/566ca04265dbbdab32ec054a"), + err: errors.New("asd"), + info: nil, + }, + { + label: "client bad response", + inputURL: utils.MustParseURL("https://betterttv.com/user/566ca04265dbbdab32ec054a"), + err: nil, + info: &imgur.GenericInfo{}, + }, + { + label: "client no error no response", + inputURL: utils.MustParseURL("https://betterttv.com/user/566ca04265dbbdab32ec054a"), + err: nil, + info: nil, + }, + } + + for _, test := range tests { + c.Run(test.label, func(c *qt.C) { + imgurClient. + EXPECT(). + GetInfoFromURL(test.inputURL.String()). + Times(1). + Return(test.info, 0, test.err) + pool.ExpectQuery("SELECT").WillReturnError(pgx.ErrNoRows) + outputBytes, outputError := r.Run(ctx, test.inputURL, test.inputReq) + c.Assert(outputError, qt.Equals, resolver.ErrDontHandle) + c.Assert(outputBytes, qt.IsNil) + }) + } + }) + + c.Run("Not cached", func(c *qt.C) { + type runTest struct { + label string + inputURL *url.URL + info *imgur.GenericInfo + expectedBytes []byte + expectedError error + rowsReturned int + } + + tests := []runTest{ + { + label: "A", + inputURL: utils.MustParseURL("https://imgur.com/a"), + info: &imgur.GenericInfo{ + Image: &imgur.ImageInfo{ + Title: "My Cool Title", + Description: "My Cool Description", + Datetime: int(datetime), + Link: "https://i.imgur.com/a.png", + }, + Album: nil, + GImage: nil, + GAlbum: nil, + Limit: &imgur.RateLimit{}, + }, + expectedBytes: []byte(`{"status":200,"thumbnail":"https://example.com/thumbnail/https%3A%2F%2Fi.imgur.com%2Fa.png","tooltip":"%3Cdiv%20style=%22text-align:%20left%3B%22%3E%3Cli%3E%3Cb%3ETitle:%3C%2Fb%3E%20My%20Cool%20Title%3C%2Fli%3E%3Cli%3E%3Cb%3EDescription:%3C%2Fb%3E%20My%20Cool%20Description%3C%2Fli%3E%3Cli%3E%3Cb%3EUploaded:%3C%2Fb%3E%2010%20Nov%202019%20%E2%80%A2%2023:00%20UTC%3C%2Fli%3E%3C%2Fdiv%3E"}`), + expectedError: nil, + }, + { + label: "Link too big", + inputURL: utils.MustParseURL("https://imgur.com/toobig"), + info: &imgur.GenericInfo{ + Image: &imgur.ImageInfo{ + Title: "My Cool Title", + Description: "My Cool Description", + Datetime: int(datetime), + Size: maxRawImageSize + 1, + Link: "https://i.imgur.com/toobig.png", + }, + Album: nil, + GImage: nil, + GAlbum: nil, + Limit: &imgur.RateLimit{}, + }, + expectedBytes: []byte(`{"status":200,"thumbnail":"https://example.com/thumbnail/https%3A%2F%2Fi.imgur.com%2Ftoobigl.png","tooltip":"%3Cdiv%20style=%22text-align:%20left%3B%22%3E%3Cli%3E%3Cb%3ETitle:%3C%2Fb%3E%20My%20Cool%20Title%3C%2Fli%3E%3Cli%3E%3Cb%3EDescription:%3C%2Fb%3E%20My%20Cool%20Description%3C%2Fli%3E%3Cli%3E%3Cb%3EUploaded:%3C%2Fb%3E%2010%20Nov%202019%20%E2%80%A2%2023:00%20UTC%3C%2Fli%3E%3C%2Fdiv%3E"}`), + expectedError: nil, + }, + { + label: "Link too big, malformed url", + inputURL: utils.MustParseURL("https://imgur.com/toobigbadurl"), + info: &imgur.GenericInfo{ + Image: &imgur.ImageInfo{ + Title: "My Cool Title", + Description: "My Cool Description", + Datetime: int(datetime), + Size: maxRawImageSize + 1, + Link: ":", + }, + Album: nil, + GImage: nil, + GAlbum: nil, + Limit: &imgur.RateLimit{}, + }, + expectedBytes: []byte(`{"status":200,"tooltip":"%3Cdiv%20style=%22text-align:%20left%3B%22%3E%3Cli%3E%3Cb%3ETitle:%3C%2Fb%3E%20My%20Cool%20Title%3C%2Fli%3E%3Cli%3E%3Cb%3EDescription:%3C%2Fb%3E%20My%20Cool%20Description%3C%2Fli%3E%3Cli%3E%3Cb%3EUploaded:%3C%2Fb%3E%2010%20Nov%202019%20%E2%80%A2%2023:00%20UTC%3C%2Fli%3E%3C%2Fdiv%3E"}`), + expectedError: nil, + }, + { + label: "Animated non-gif", + inputURL: utils.MustParseURL("https://imgur.com/b"), + info: &imgur.GenericInfo{ + Image: &imgur.ImageInfo{ + Title: "My Cool Title", + Description: "My Cool Description", + Datetime: int(datetime), + Link: "https://i.imgur.com/b.mp4", + Animated: true, + }, + Album: nil, + GImage: nil, + GAlbum: nil, + Limit: &imgur.RateLimit{}, + }, + expectedBytes: []byte(`{"status":200,"thumbnail":"https://example.com/thumbnail/https%3A%2F%2Fi.imgur.com%2Fb.png","tooltip":"%3Cdiv%20style=%22text-align:%20left%3B%22%3E%3Cli%3E%3Cb%3ETitle:%3C%2Fb%3E%20My%20Cool%20Title%3C%2Fli%3E%3Cli%3E%3Cb%3EDescription:%3C%2Fb%3E%20My%20Cool%20Description%3C%2Fli%3E%3Cli%3E%3Cb%3EUploaded:%3C%2Fb%3E%2010%20Nov%202019%20%E2%80%A2%2023:00%20UTC%3C%2Fli%3E%3Cli%3E%3Cb%3E%3Cspan%20style=%22color:%20red%3B%22%3EANIMATED%3C%2Fspan%3E%3C%2Fb%3E%3C%2Fli%3E%3C%2Fdiv%3E"}`), + expectedError: nil, + }, + { + label: "Animated malformed link", + inputURL: utils.MustParseURL("https://imgur.com/c"), + info: &imgur.GenericInfo{ + Image: &imgur.ImageInfo{ + Title: "My Cool Title", + Description: "My Cool Description", + Datetime: int(datetime), + Link: ":", + Animated: true, + }, + Album: nil, + GImage: nil, + GAlbum: nil, + Limit: &imgur.RateLimit{}, + }, + expectedBytes: []byte(`{"status":200,"tooltip":"%3Cdiv%20style=%22text-align:%20left%3B%22%3E%3Cli%3E%3Cb%3ETitle:%3C%2Fb%3E%20My%20Cool%20Title%3C%2Fli%3E%3Cli%3E%3Cb%3EDescription:%3C%2Fb%3E%20My%20Cool%20Description%3C%2Fli%3E%3Cli%3E%3Cb%3EUploaded:%3C%2Fb%3E%2010%20Nov%202019%20%E2%80%A2%2023:00%20UTC%3C%2Fli%3E%3Cli%3E%3Cb%3E%3Cspan%20style=%22color:%20red%3B%22%3EANIMATED%3C%2Fspan%3E%3C%2Fb%3E%3C%2Fli%3E%3C%2Fdiv%3E"}`), + expectedError: nil, + }, + } + + for _, test := range tests { + c.Run(test.label, func(c *qt.C) { + imgurClient. + EXPECT(). + GetInfoFromURL(test.inputURL.String()). + Times(1). + Return(test.info, 0, nil) + pool.ExpectQuery("SELECT").WillReturnError(pgx.ErrNoRows) + pool.ExpectExec("INSERT INTO cache"). + WithArgs("imgur:"+test.inputURL.String(), test.expectedBytes, pgxmock.AnyArg()). + WillReturnResult(pgxmock.NewResult("INSERT", 1)) + outputBytes, outputError := r.Run(ctx, test.inputURL, nil) + c.Assert(outputError, qt.Equals, test.expectedError) + c.Assert(outputBytes, qt.DeepEquals, test.expectedBytes) + }) + } + }) + }) +}