From 1474a577eae5dabb13e0cd8b98f2dd55d85d0f70 Mon Sep 17 00:00:00 2001 From: Ryan Fowler Date: Thu, 12 Feb 2026 17:59:09 -0800 Subject: [PATCH] Introduce formatter registry to eliminate formatResponse() god function Move ContentType enum, GetContentType(), and SniffContentType() into the format package and add a registry pattern with BufferedFormatter and StreamingFormatter types. Each formatter self-registers via init(), replacing the hardcoded switch statements in formatResponse() with registry lookups. Special cases (gRPC streaming, Protobuf with descriptor, Image rendering) remain inline due to extra context needs. --- internal/fetch/charset_test.go | 62 ------- internal/fetch/fetch.go | 164 +++--------------- .../{fetch/sniff.go => format/contenttype.go} | 95 +++++++++- .../contenttype_test.go} | 68 +++++++- internal/format/registry.go | 45 +++++ 5 files changed, 228 insertions(+), 206 deletions(-) rename internal/{fetch/sniff.go => format/contenttype.go} (60%) rename internal/{fetch/sniff_test.go => format/contenttype_test.go} (72%) create mode 100644 internal/format/registry.go diff --git a/internal/fetch/charset_test.go b/internal/fetch/charset_test.go index 8c0af58..ead1af2 100644 --- a/internal/fetch/charset_test.go +++ b/internal/fetch/charset_test.go @@ -2,7 +2,6 @@ package fetch import ( "io" - "net/http" "strings" "testing" ) @@ -124,64 +123,3 @@ func TestTranscodeReader(t *testing.T) { }) } } - -func TestGetContentTypeCharset(t *testing.T) { - tests := []struct { - name string - contentType string - wantType ContentType - wantCharset string - }{ - { - name: "json with charset", - contentType: "application/json; charset=utf-8", - wantType: TypeJSON, - wantCharset: "utf-8", - }, - { - name: "html with charset", - contentType: "text/html; charset=iso-8859-1", - wantType: TypeHTML, - wantCharset: "iso-8859-1", - }, - { - name: "json without charset", - contentType: "application/json", - wantType: TypeJSON, - wantCharset: "", - }, - { - name: "empty content type", - contentType: "", - wantType: TypeUnknown, - wantCharset: "", - }, - { - name: "xml with charset", - contentType: "text/xml; charset=windows-1252", - wantType: TypeXML, - wantCharset: "windows-1252", - }, - { - name: "csv with charset", - contentType: "text/csv; charset=shift_jis", - wantType: TypeCSV, - wantCharset: "shift_jis", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - headers := http.Header{} - if tt.contentType != "" { - headers.Set("Content-Type", tt.contentType) - } - gotType, gotCharset := getContentType(headers) - if gotType != tt.wantType { - t.Errorf("getContentType() type = %v, want %v", gotType, tt.wantType) - } - if gotCharset != tt.wantCharset { - t.Errorf("getContentType() charset = %q, want %q", gotCharset, tt.wantCharset) - } - }) - } -} diff --git a/internal/fetch/fetch.go b/internal/fetch/fetch.go index 8e65f9a..4e440ee 100644 --- a/internal/fetch/fetch.go +++ b/internal/fetch/fetch.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "io" - "mime" "net/http" "net/url" "os" @@ -33,24 +32,6 @@ import ( // formatting a response body or copying it to the clipboard. const maxBodyBytes = 1 << 20 // 1MiB -type ContentType int - -const ( - TypeUnknown ContentType = iota - TypeCSS - TypeCSV - TypeGRPC - TypeHTML - TypeImage - TypeJSON - TypeMsgPack - TypeNDJSON - TypeProtobuf - TypeSSE - TypeXML - TypeYAML -) - type Request struct { AWSSigv4 *aws.Config Basic *core.KeyVal[string] @@ -390,21 +371,20 @@ func formatResponse(ctx context.Context, r *Request, resp *http.Response) (io.Re } p := r.PrinterHandle.Stdout() - contentType, charset := getContentType(resp.Header) - switch contentType { - case TypeGRPC: - // NOTE: This bypasses the isPrintable check for binary data. + contentType, charset := format.GetContentType(resp.Header) + + // gRPC streaming needs the response descriptor — handle inline. + if contentType == format.TypeGRPC { return nil, format.FormatGRPCStream(resp.Body, r.responseDescriptor, p) - case TypeNDJSON: - // NOTE: This bypasses the isPrintable check for binary data. - return nil, format.FormatNDJSON(transcodeReader(resp.Body, charset), p) - case TypeSSE: - // NOTE: This bypasses the isPrintable check for binary data. - return nil, format.FormatEventStream(transcodeReader(resp.Body, charset), p) + } + + // Dispatch registered streaming formatters (NDJSON, SSE). + if fn := format.GetStreaming(contentType); fn != nil { + return nil, fn(transcodeReader(resp.Body, charset), p) } // If image rendering is disabled, return the reader immediately. - if contentType == TypeImage && r.Image == core.ImageOff { + if contentType == format.TypeImage && r.Image == core.ImageOff { return resp.Body, nil } @@ -418,134 +398,42 @@ func formatResponse(ctx context.Context, r *Request, resp *http.Response) (io.Re } // If the Content-Type is unknown, attempt to sniff the body. - if contentType == TypeUnknown { - contentType = sniffContentType(buf) - if contentType == TypeUnknown { + if contentType == format.TypeUnknown { + contentType = format.SniffContentType(buf) + if contentType == format.TypeUnknown { return bytes.NewReader(buf), nil } - if contentType == TypeImage && r.Image == core.ImageOff { + if contentType == format.TypeImage && r.Image == core.ImageOff { return bytes.NewReader(buf), nil } } // Transcode non-UTF-8 text to UTF-8, skipping binary formats. switch contentType { - case TypeImage, TypeMsgPack, TypeProtobuf: + case format.TypeImage, format.TypeMsgPack, format.TypeProtobuf: default: buf = transcodeBytes(buf, charset) } - switch contentType { - case TypeCSS: - if format.FormatCSS(buf, p) == nil { - buf = p.Bytes() - } - case TypeCSV: - if format.FormatCSV(buf, p) == nil { - buf = p.Bytes() - } - case TypeHTML: - if format.FormatHTML(buf, p) == nil { - buf = p.Bytes() - } - case TypeImage: + // Special cases that need extra context beyond ([]byte, *Printer). + if contentType == format.TypeImage { return nil, image.Render(ctx, buf, r.Image == core.ImageNative) - case TypeJSON: - if format.FormatJSON(buf, p) == nil { - buf = p.Bytes() - } - case TypeMsgPack: - if format.FormatMsgPack(buf, p) == nil { - buf = p.Bytes() - } - case TypeProtobuf: - var err error - if r.responseDescriptor != nil { - err = format.FormatProtobufWithDescriptor(buf, r.responseDescriptor, p) - } else { - err = format.FormatProtobuf(buf, p) - } - if err == nil { - buf = p.Bytes() - } - case TypeXML: - if format.FormatXML(buf, p) == nil { - buf = p.Bytes() - } - case TypeYAML: - if format.FormatYAML(buf, p) == nil { + } + if contentType == format.TypeProtobuf && r.responseDescriptor != nil { + if format.FormatProtobufWithDescriptor(buf, r.responseDescriptor, p) == nil { buf = p.Bytes() } + return bytes.NewReader(buf), nil } - return bytes.NewReader(buf), nil -} - -func getContentType(headers http.Header) (ContentType, string) { - contentType := headers.Get("Content-Type") - if contentType == "" { - return TypeUnknown, "" - } - mediaType, params, err := mime.ParseMediaType(contentType) - if err != nil { - return TypeUnknown, "" - } - charset := params["charset"] - - if typ, subtype, ok := strings.Cut(mediaType, "/"); ok { - switch typ { - case "image": - return TypeImage, charset - case "application": - switch subtype { - case "csv": - return TypeCSV, charset - case "grpc", "grpc+proto": - return TypeGRPC, charset - case "json": - return TypeJSON, charset - case "msgpack", "x-msgpack", "vnd.msgpack": - return TypeMsgPack, charset - case "x-ndjson", "ndjson", "x-jsonl", "jsonl", "x-jsonlines": - return TypeNDJSON, charset - case "protobuf", "x-protobuf", "x-google-protobuf", "vnd.google.protobuf": - return TypeProtobuf, charset - case "xml": - return TypeXML, charset - case "yaml", "x-yaml": - return TypeYAML, charset - } - if strings.HasSuffix(subtype, "+json") || strings.HasSuffix(subtype, "-json") { - return TypeJSON, charset - } - if strings.HasSuffix(subtype, "+proto") { - return TypeProtobuf, charset - } - if strings.HasSuffix(subtype, "+xml") { - return TypeXML, charset - } - if strings.HasSuffix(subtype, "+yaml") { - return TypeYAML, charset - } - case "text": - switch subtype { - case "css": - return TypeCSS, charset - case "csv": - return TypeCSV, charset - case "html": - return TypeHTML, charset - case "event-stream": - return TypeSSE, charset - case "xml": - return TypeXML, charset - case "yaml", "x-yaml": - return TypeYAML, charset - } + // Dispatch registered buffered formatters. + if fn := format.GetBuffered(contentType); fn != nil { + if fn(buf, p) == nil { + buf = p.Bytes() } } - return TypeUnknown, charset + return bytes.NewReader(buf), nil } func streamToStdout(r io.Reader, p *core.Printer, forceOutput, noPager bool) error { diff --git a/internal/fetch/sniff.go b/internal/format/contenttype.go similarity index 60% rename from internal/fetch/sniff.go rename to internal/format/contenttype.go index 25b285d..2fbb98b 100644 --- a/internal/fetch/sniff.go +++ b/internal/format/contenttype.go @@ -1,11 +1,100 @@ -package fetch +package format import ( "bytes" + "mime" "net/http" "strings" ) +// ContentType represents a recognized response content type. +type ContentType int + +const ( + TypeUnknown ContentType = iota + TypeCSS + TypeCSV + TypeGRPC + TypeHTML + TypeImage + TypeJSON + TypeMsgPack + TypeNDJSON + TypeProtobuf + TypeSSE + TypeXML + TypeYAML +) + +// GetContentType parses the Content-Type header and returns the detected +// ContentType and charset. Returns TypeUnknown if the type is not recognized. +func GetContentType(headers http.Header) (ContentType, string) { + contentType := headers.Get("Content-Type") + if contentType == "" { + return TypeUnknown, "" + } + mediaType, params, err := mime.ParseMediaType(contentType) + if err != nil { + return TypeUnknown, "" + } + charset := params["charset"] + + if typ, subtype, ok := strings.Cut(mediaType, "/"); ok { + switch typ { + case "image": + return TypeImage, charset + case "application": + switch subtype { + case "csv": + return TypeCSV, charset + case "grpc", "grpc+proto": + return TypeGRPC, charset + case "json": + return TypeJSON, charset + case "msgpack", "x-msgpack", "vnd.msgpack": + return TypeMsgPack, charset + case "x-ndjson", "ndjson", "x-jsonl", "jsonl", "x-jsonlines": + return TypeNDJSON, charset + case "protobuf", "x-protobuf", "x-google-protobuf", "vnd.google.protobuf": + return TypeProtobuf, charset + case "xml": + return TypeXML, charset + case "yaml", "x-yaml": + return TypeYAML, charset + } + if strings.HasSuffix(subtype, "+json") || strings.HasSuffix(subtype, "-json") { + return TypeJSON, charset + } + if strings.HasSuffix(subtype, "+proto") { + return TypeProtobuf, charset + } + if strings.HasSuffix(subtype, "+xml") { + return TypeXML, charset + } + if strings.HasSuffix(subtype, "+yaml") { + return TypeYAML, charset + } + case "text": + switch subtype { + case "css": + return TypeCSS, charset + case "csv": + return TypeCSV, charset + case "html": + return TypeHTML, charset + case "event-stream": + return TypeSSE, charset + case "xml": + return TypeXML, charset + case "yaml", "x-yaml": + return TypeYAML, charset + } + } + } + + return TypeUnknown, charset +} + var ( prefixXMLDecl = []byte("?xml") prefixDoctype = []byte("!doctype") @@ -27,10 +116,10 @@ var ( } ) -// sniffContentType attempts to detect the content type of the provided bytes +// SniffContentType attempts to detect the content type of the provided bytes // by examining the leading bytes of the body. It is used as a fallback when // no Content-Type header is present in the HTTP response. -func sniffContentType(buf []byte) ContentType { +func SniffContentType(buf []byte) ContentType { b := trimBOMAndSpace(buf) if len(b) == 0 { return TypeUnknown diff --git a/internal/fetch/sniff_test.go b/internal/format/contenttype_test.go similarity index 72% rename from internal/fetch/sniff_test.go rename to internal/format/contenttype_test.go index 14b433f..ca1af9a 100644 --- a/internal/fetch/sniff_test.go +++ b/internal/format/contenttype_test.go @@ -1,6 +1,7 @@ -package fetch +package format import ( + "net/http" "testing" ) @@ -66,9 +67,9 @@ func TestSniffContentType(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := sniffContentType(tt.input) + got := SniffContentType(tt.input) if got != tt.want { - t.Errorf("sniffContentType(%q) = %d, want %d", tt.input, got, tt.want) + t.Errorf("SniffContentType(%q) = %d, want %d", tt.input, got, tt.want) } }) } @@ -126,3 +127,64 @@ func TestIsLetter(t *testing.T) { }) } } + +func TestGetContentType(t *testing.T) { + tests := []struct { + name string + contentType string + wantType ContentType + wantCharset string + }{ + { + name: "json with charset", + contentType: "application/json; charset=utf-8", + wantType: TypeJSON, + wantCharset: "utf-8", + }, + { + name: "html with charset", + contentType: "text/html; charset=iso-8859-1", + wantType: TypeHTML, + wantCharset: "iso-8859-1", + }, + { + name: "json without charset", + contentType: "application/json", + wantType: TypeJSON, + wantCharset: "", + }, + { + name: "empty content type", + contentType: "", + wantType: TypeUnknown, + wantCharset: "", + }, + { + name: "xml with charset", + contentType: "text/xml; charset=windows-1252", + wantType: TypeXML, + wantCharset: "windows-1252", + }, + { + name: "csv with charset", + contentType: "text/csv; charset=shift_jis", + wantType: TypeCSV, + wantCharset: "shift_jis", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + headers := http.Header{} + if tt.contentType != "" { + headers.Set("Content-Type", tt.contentType) + } + gotType, gotCharset := GetContentType(headers) + if gotType != tt.wantType { + t.Errorf("GetContentType() type = %v, want %v", gotType, tt.wantType) + } + if gotCharset != tt.wantCharset { + t.Errorf("GetContentType() charset = %q, want %q", gotCharset, tt.wantCharset) + } + }) + } +} diff --git a/internal/format/registry.go b/internal/format/registry.go new file mode 100644 index 0000000..139d61c --- /dev/null +++ b/internal/format/registry.go @@ -0,0 +1,45 @@ +package format + +import ( + "io" + + "github.com/ryanfowler/fetch/internal/core" +) + +// BufferedFormatter formats buffered bytes to the Printer. +type BufferedFormatter func(buf []byte, p *core.Printer) error + +// StreamingFormatter formats a stream to the Printer. +type StreamingFormatter func(r io.Reader, p *core.Printer) error + +var ( + bufferedFormatters map[ContentType]BufferedFormatter + streamingFormatters map[ContentType]StreamingFormatter +) + +func init() { + bufferedFormatters = map[ContentType]BufferedFormatter{ + TypeCSS: FormatCSS, + TypeCSV: FormatCSV, + TypeHTML: FormatHTML, + TypeJSON: FormatJSON, + TypeMsgPack: FormatMsgPack, + TypeProtobuf: FormatProtobuf, + TypeXML: FormatXML, + TypeYAML: FormatYAML, + } + streamingFormatters = map[ContentType]StreamingFormatter{ + TypeNDJSON: FormatNDJSON, + TypeSSE: FormatEventStream, + } +} + +// GetBuffered returns the buffered formatter for the given content type, or nil. +func GetBuffered(ct ContentType) BufferedFormatter { + return bufferedFormatters[ct] +} + +// GetStreaming returns the streaming formatter for the given content type, or nil. +func GetStreaming(ct ContentType) StreamingFormatter { + return streamingFormatters[ct] +}