Update linting and simplify code

Author: ahobsonsayers

.golangci.yaml

@@ -8,27 +8,32 @@ linters:
     # Default linters: https://golangci-lint.run/usage/linters#enabled-by-default
     - gofmt
     - goimports
+    - makezero
+    - misspell
     - revive
     - stylecheck
-    - misspell
     - unparam
-    - makezero
 
 linters-settings:
+  # https://golangci-lint.run/usage/linters#revive
   revive:
-    # https://golangci-lint.run/usage/linters#revive
+    enable-all-rules: true
     rules:
+      - name: add-constant
+        disabled: true
+
+      - name: line-length-limit
+        arguments: [120]
+
       - name: var-naming
         arguments:
           - ["ID", "JSON", "URL"] # Allow List
           - [] # Deny List
 
+  # https://golangci-lint.run/usage/linters/#stylecheck
   stylecheck:
-    # https://golangci-lint.run/usage/linters/#stylecheck
-    checks:
-      - "*"
-      - "-ST1003" # Duplicate check of var-naming from revive
+    checks: ["all", "-ST1003"] # ST1003 is a duplicate check of var-naming from revive
 
+  # https://golangci-lint.run/usage/linters/#unparam
   unparam:
-    # https://golangci-lint.run/usage/linters/#unparam
     check-exported: true

goodreads/book.go

@@ -72,6 +72,21 @@ type Edition struct {
 	LanguageCode     string  `xml:"language_code"`
 }
 
+// func (e *Edition) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
+// 	type alias Edition
+// 	var unmarshaller alias
+// 	err := d.DecodeElement(&unmarshaller, &start)
+// 	if err != nil {
+// 		return err
+// 	}
+// 	*e = Edition(unmarshaller)
+
+// 	// Cleanup some fields
+// 	e.Description = html2text.HTML2Text(e.Description)
+
+// 	return nil
+// }
+
 type SeriesBook struct {
 	Series       Series  `xml:"series"`
 	BookPosition *string `xml:"user_position"`

goodreads/book_test.go

@@ -34,6 +34,6 @@ func TestUnmarshalGenres(t *testing.T) {
 	err := xml.Unmarshal([]byte(xmlString), &genres)
 	require.NoError(t, err)
 
-	expectedGenres := goodreads.Genres{"Fantasy", "Classic", "Fiction", "Adventure", "Young Adult"}
+	expectedGenres := goodreads.Genres{"Fantasy", "Classic", "Fiction"}
 	require.Equal(t, expectedGenres, genres)
 }

goodreads/client.go

@@ -17,19 +17,19 @@ const (
 	DefaultAPIKey     = "ckvsiSDsuqh7omh74ZZ6Q" // Read only API key kindly provided by LazyLibrarian
 )
 
-var DefaultGoodreadsClient = &GoodreadsClient{
+var DefaultClient = &Client{
 	client:     http.DefaultClient,
 	apiRootUrl: DefaultAPIRootUrl,
 	apiKey:     DefaultAPIKey,
 }
 
-type GoodreadsClient struct {
+type Client struct {
 	client     *http.Client
 	apiRootUrl string
 	apiKey     string
 }
 
-func (c *GoodreadsClient) Get(
+func (c *Client) Get(
 	ctx context.Context,
 	apiPath string,
 	queryParams map[string]string,
@@ -82,7 +82,7 @@ func (c *GoodreadsClient) Get(
 
 // GetBookById gets a book by its id.
 // https://www.goodreads.com/api/index#book.show
-func (c *GoodreadsClient) GetBookById(ctx context.Context, bookId string) (Book, error) {
+func (c *Client) GetBookById(ctx context.Context, bookId string) (Book, error) {
 	queryParams := map[string]string{"id": bookId}
 
 	var result struct {
@@ -96,9 +96,36 @@ func (c *GoodreadsClient) GetBookById(ctx context.Context, bookId string) (Book,
 	return result.Book, nil
 }
 
+func (c *Client) GetBooksByIds(ctx context.Context, bookIds []string) ([]Book, error) {
+	books := make([]Book, len(bookIds))
+	var errs error
+	var wg sync.WaitGroup
+	for idx, bookId := range bookIds {
+		wg.Add(1)
+		go func(bookId string, idx int) {
+			defer wg.Done()
+
+			book, err := c.GetBookById(ctx, bookId)
+			if err != nil {
+				errs = errors.Join(errs, err)
+				return
+			}
+			books[idx] = book
+		}(bookId, idx)
+	}
+
+	wg.Wait()
+
+	if errs != nil {
+		return nil, errs
+	}
+
+	return books, nil
+}
+
 // GetBookByTitle gets a book by its title and optionally an author (which can give a better match)
 // https://www.goodreads.com/api/index#book.title
-func (c *GoodreadsClient) GetBookByTitle(ctx context.Context, bookTitle string, bookAuthor *string) (Book, error) {
+func (c *Client) GetBookByTitle(ctx context.Context, bookTitle string, bookAuthor *string) (Book, error) {
 	queryParams := map[string]string{"title": bookTitle}
 	if bookAuthor != nil && *bookAuthor != "" {
 		queryParams["author"] = *bookAuthor
@@ -117,40 +144,26 @@ func (c *GoodreadsClient) GetBookByTitle(ctx context.Context, bookTitle string,
 
 // SearchBooks search for a book by its title and optionally an author (which can give better results)
 // https://www.goodreads.com/api/index#search.books
-func (c *GoodreadsClient) SearchBooks(ctx context.Context, bookTitle string, bookAuthor *string) ([]Book, error) {
+func (c *Client) SearchBooks(ctx context.Context, bookTitle string, bookAuthor *string) ([]Book, error) {
 	query := bookTitle
 	if bookAuthor != nil && *bookAuthor != "" {
 		query = fmt.Sprintf("%s %s", query, *bookAuthor)
 	}
 	queryParams := map[string]string{"q": query}
 
-	var result struct {
+	// Search for books, getting their ids
+	var unmarshaller struct {
 		BookIds []string `xml:"search>results>work>best_book>id"`
 	}
-	err := c.Get(ctx, "search/index.xml", queryParams, &result)
+	err := c.Get(ctx, "search/index.xml", queryParams, &unmarshaller)
 	if err != nil {
 		return nil, err
 	}
 
-	books := make([]Book, len(result.BookIds))
-	var errs error
-	var wg sync.WaitGroup
-	for idx, bookId := range result.BookIds {
-		wg.Add(1)
-		go func(bookId string, idx int) {
-			defer wg.Done()
-
-			book, err := c.GetBookById(ctx, bookId)
-			if err != nil {
-				errs = errors.Join(errs, err)
-				return
-			}
-			books[idx] = book
-		}(bookId, idx)
-	}
-	wg.Wait()
-	if errs != nil {
-		return nil, errs
+	// Get book details using their ids
+	books, err := c.GetBooksByIds(ctx, unmarshaller.BookIds)
+	if err != nil {
+		return nil, err
 	}
 
 	return books, nil

goodreads/error.go

@@ -15,7 +15,7 @@ func HTTPResponseError(response *http.Response) error {
 			return fmt.Errorf("got status %s", response.Status)
 		}
 
-		var responseContent interface{}
+		var responseContent any
 		err = json.Unmarshal(responseBody, &responseContent)
 		if err != nil {
 			responseContent = string(responseBody)

goodreads/genre.go

@@ -73,45 +73,59 @@ var genreShelves = mapset.NewSet(
 type Genres []string
 
 func (g *Genres) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
-	// Shelves is a struct matching the goodread response xml
-	var shelves struct {
-		Shelf []struct {
+	// unmarshaller is a struct matching the goodreads response xml
+	var unmarshaller struct {
+		Shelves []struct { // nolint
 			Name string `xml:"name,attr"`
 		} `xml:"shelf"`
 	}
-	err := d.DecodeElement(&shelves, &start)
+	err := d.DecodeElement(&unmarshaller, &start)
 	if err != nil {
 		return err
 	}
 
-	genres := make(Genres, 0, 3)
-	seenGenreShelves := mapset.NewSetWithSize[string](3)
-	for _, shelf := range shelves.Shelf {
-		// Make shelf name singular for easier comparison
-		shelfName := inflection.Singular(shelf.Name)
+	// Get shelf names
+	shelfNames := make([]string, 0, len(unmarshaller.Shelves))
+	for _, shelf := range unmarshaller.Shelves {
+		shelfNames = append(shelfNames, shelf.Name)
+	}
+
+	// Convert shelf names to genres
+	genres := shelvesToGenres(shelfNames)
+
+	// Only use first (up to) three genres
+	if len(genres) < 3 {
+		*g = genres
+	} else {
+		*g = genres[:3]
+	}
+
+	return nil
+}
+
+func shelvesToGenres(shelves []string) []string {
+	genres := make(Genres, 0, len(shelves))
+	seenGenreShelves := mapset.NewSetWithSize[string](len(shelves))
+	for _, shelf := range shelves {
+		// Make shelf singular for easier comparison
+		shelf := inflection.Singular(shelf)
 
 		// Skip non genre shelves and already seen genre shelves
-		if !genreShelves.Contains(shelfName) || seenGenreShelves.Contains(shelfName) {
+		if !genreShelves.Contains(shelf) || seenGenreShelves.Contains(shelf) {
 			continue
 		}
 
 		// Get genre from shelf name, making it human readable
-		genre := strings.ReplaceAll(shelfName, "-", " ")
+		genre := strings.ReplaceAll(shelf, "-", " ")
 		genreWords := strings.Fields(genre)
 		for i, word := range genreWords {
 			genreWords[i] = cases.Title(language.Und).String(word) // Capitalize each word
 		}
 		genre = strings.Join(genreWords, " ")
 
 		genres = append(genres, genre)
-		seenGenreShelves.Add(shelfName)
-
-		// Only get top 3 genres
-		if len(genres) == 3 {
-			break
-		}
+		seenGenreShelves.Add(shelf)
 	}
-	*g = genres
 
-	return nil
+	return genres
 }

goodreads/new.go

@@ -8,7 +8,7 @@ import (
 // NewClient creates a new goodreads client.
 // If client is nil, the default http client will be used.
 // If api url is nil or uset, the default goodreads api url will be used
-func NewClient(client *http.Client, apiURL *string, apiKey *string) *GoodreadsClient {
+func NewClient(client *http.Client, apiURL *string, apiKey *string) *Client {
 	if client == nil {
 		client = http.DefaultClient
 	}
@@ -23,7 +23,7 @@ func NewClient(client *http.Client, apiURL *string, apiKey *string) *GoodreadsCl
 		apiKeyString = strings.TrimSpace(*apiKey)
 	}
 
-	return &GoodreadsClient{
+	return &Client{
 		client:     client,
 		apiRootUrl: apiUrlString,
 		apiKey:     apiKeyString,

server/server.go

@@ -13,7 +13,7 @@ func NewServer() StrictServerInterface { return &server{} }
 
 func (*server) Search(ctx context.Context, request SearchRequestObject) (SearchResponseObject, error) {
 	// Search book
-	goodreadsBooks, err := goodreads.DefaultGoodreadsClient.SearchBooks(ctx, request.Params.Query, request.Params.Author)
+	goodreadsBooks, err := goodreads.DefaultClient.SearchBooks(ctx, request.Params.Query, request.Params.Author)
 	if err != nil {
 		return Search500JSONResponse{Error: utils.ToPointer(err.Error())}, nil
 	}