Skip to content

Conversation

cnlangzi
Copy link
Member

@cnlangzi cnlangzi commented Jan 19, 2025

Changed

  • improved accept match rules

Fixed

Added

Tests

Tasks to complete before merging PR:

  • Ensure unit tests are passing. If not run make unit-test to check for any regressions 📋
  • Ensure lint tests are passing. if not run make lint to check for any issues
  • Ensure codecov/patch is passing for changes.

Summary by Sourcery

Improve MIME type matching rules and update the View method signature. Use the first matching viewer based on the Accept header, or the first available viewer if no match is found.

Bug Fixes:

  • Fixed an issue with MIME type matching

Enhancements:

  • Simplified the View method signature to accept an optional list of viewer names.

Copy link

sourcery-ai bot commented Jan 19, 2025

Reviewer's Guide by Sourcery

This pull request improves the matching logic for MIME types and refactors the view rendering process to use the new matching logic. It also introduces a MimeType struct to represent MIME types.

Sequence diagram for improved view matching process

sequenceDiagram
    participant C as Context
    participant V as Viewer
    participant MT as MimeType

    C->>C: View(data, options...)
    C->>C: getViewer(name)
    alt Named viewer exists
        C->>V: MimeType()
        V-->>C: viewer mime type
        C->>MT: Match(accept)
        MT-->>C: match result
    else No named viewer
        loop For each Accept header
            C->>V: MimeType()
            V-->>C: viewer mime type
            C->>MT: Match(accept)
            MT-->>C: match result
        end
    end
    C->>V: Render(response, request, data)
Loading

Class diagram for the new MimeType structure and Viewer interface changes

classDiagram
    class MimeType {
        +String Type
        +String SubType
        +Match(accept MimeType) bool
        +String() string
    }

    class Viewer {
        <<interface>>
        +MimeType() *MimeType
        +Render(w http.ResponseWriter, r *http.Request, data any) error
    }

    class JsonViewer {
        +MimeType() *MimeType
        +Render(w http.ResponseWriter, r *http.Request, data any) error
    }

    class HtmlViewer {
        +MimeType() *MimeType
        +Render(w http.ResponseWriter, r *http.Request, data any) error
    }

    class FileViewer {
        +MimeType() *MimeType
        +Render(w http.ResponseWriter, r *http.Request, data any) error
    }

    Viewer <|.. JsonViewer
    Viewer <|.. HtmlViewer
    Viewer <|.. FileViewer

    note for MimeType "New struct for MIME type handling"
    note for Viewer "Updated to return *MimeType"
Loading

File-Level Changes

Change Details Files
Introduced a MimeType struct to represent MIME types.
  • Added a MimeType struct with Type and SubType fields.
  • Added a NewMimeType function to create a MimeType from a string.
  • Added a Match function to check if two MimeTypes match.
  • Added a String function to return the string representation of a MimeType.
mime.go
Refactored the Accept method to return a slice of MimeType instead of string.
  • Modified the Accept method to return a slice of MimeType.
  • Updated the Accept method to use the NewMimeType function.
context.go
Refactored the View method to use the new MimeType matching logic.
  • Modified the View method to accept an optional list of viewer names.
  • Updated the View method to use the MimeType.Match method to find a matching viewer.
  • Updated the View method to use the first viewer in the list if no viewer is matched.
context.go
Updated the GetMimeType function to return a MimeType struct.
  • Modified the GetMimeType function to return a MimeType struct.
  • Updated the GetMimeType function to use the NewMimeType function.
mime.go
Updated the Viewer interface to use MimeType struct.
  • Modified the Viewer interface to use MimeType struct.
  • Updated the FileViewer to use a MimeType struct.
  • Updated the HtmlViewer to use a MimeType struct.
  • Updated the JsonViewer to use a MimeType struct.
  • Updated the TextViewer to use a MimeType struct.
viewer.go
viewer_file.go
viewer_html.go
viewer_json.go
viewer_text.go
Updated the HandlePage and HandleFile methods to store viewers as a slice instead of a map.
  • Modified the HandlePage and HandleFile methods to store viewers as a slice.
  • Updated the HandlePage and HandleFile methods to append viewers to the slice.
app.go
Added unit tests for the MimeType matching logic.
  • Added unit tests for the MimeType.Match method.
  • Added tests for wildcard matching.
mime_test.go
Added unit tests for the View method with multiple viewers.
  • Added unit tests for the View method with multiple viewers.
  • Added tests for matching viewers based on the Accept header.
app_test.go

Assessment against linked issues

Issue Objective Addressed Explanation
#5 Separate DefaultView and View functions to improve type safety The PR does not create separate DefaultView and View functions as suggested in the issue. Instead, it modifies the existing View function to have more flexible signature and improved matching rules.
#5 Provide a clear, type-safe way to render views with optional view names

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

deepsource-io bot commented Jan 19, 2025

Here's the code health analysis summary for commits 8ffc9c5..830da26. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource Go LogoGo✅ SuccessView Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @cnlangzi - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link

codecov bot commented Jan 19, 2025

Codecov Report

Attention: Patch coverage is 98.71795% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.98%. Comparing base (8ffc9c5) to head (830da26).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
app.go 93.75% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #25      +/-   ##
==========================================
+ Coverage   89.79%   89.98%   +0.18%     
==========================================
  Files          32       32              
  Lines        1176     1198      +22     
==========================================
+ Hits         1056     1078      +22     
  Misses         83       83              
  Partials       37       37              
Flag Coverage Δ
Unit-Tests 89.98% <98.71%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cnlangzi cnlangzi merged commit 6dc74f4 into main Jan 19, 2025
7 checks passed
@cnlangzi cnlangzi deleted the fix/accept branch January 19, 2025 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DefaultView / View should be type-safe
1 participant