Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,28 @@ jobs:
matrix:
go: [ '1.21', '1.20', '1.19', '1.18']
steps:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
- name: Checkout Code
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1

- uses: actions/setup-go@93397bea11091df50f3d7e59dc26a7711a8bcfbe # v4.1.0
- name: Setup Go
uses: actions/setup-go@93397bea11091df50f3d7e59dc26a7711a8bcfbe # v4.1.0
with:
go-version: ${{ matrix.go }}

- name: test
run: go test -race . -v
- name: Run golangci-lint
uses: golangci/golangci-lint-action@08e2f20817b15149a52b5b3ebe7de50aff2ba8c5

- name: run test and generate coverage report
run: go test -race ./... -v -coverprofile=coverage.out

- name: Upload coverage report
uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808
with:
path: coverage.out
name: Coverage-report-${{matrix.go}}

- name: Display coverage report
run: go tool cover -func=coverage.out

- name: Build Go
run: go build ./...
6 changes: 6 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
linters:
enable:
- errcheck
disable:
- unused
output_format: colored-line-number
Copy link

Choose a reason for hiding this comment

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

Suggestion: new lint

Copy link

Choose a reason for hiding this comment

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

Also, why did we need to disable unused ?

4 changes: 3 additions & 1 deletion errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ func TestMarshalErrorsWritesTheExpectedPayload(t *testing.T) {
var writer io.Writer = buffer

_ = MarshalErrors(writer, testRow.In)
json.Unmarshal(buffer.Bytes(), &output)
if err := json.Unmarshal(buffer.Bytes(), &output); err != nil {
t.Fatalf("failed to unmarshal: %v", err)
}

if !reflect.DeepEqual(output, testRow.Out) {
t.Fatalf("Expected: \n%#v \nto equal: \n%#v", output, testRow.Out)
Expand Down
2 changes: 1 addition & 1 deletion examples/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func TestHttpErrorWhenHeaderDoesNotMatch(t *testing.T) {
}

func TestHttpErrorWhenMethodDoesNotMatch(t *testing.T) {
r, err := http.NewRequest(http.MethodPatch, "/blogs", nil)
r, err := http.NewRequest(http.MethodOptions, "/blogs", nil)

Choose a reason for hiding this comment

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

This test is currently failing in the main branch. It is failing because previously there wasn't any handling for the method http.MethodPatch but around an year back code was added for this case as well.
I have changed the http method to an unhandled path, so that the test case runs as expected

if err != nil {
t.Fatal(err)
}
Expand Down
12 changes: 9 additions & 3 deletions request.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,12 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node)

buf := bytes.NewBuffer(nil)

json.NewEncoder(buf).Encode(data.Relationships[args[1]])
json.NewDecoder(buf).Decode(relationship)
if err := json.NewEncoder(buf).Encode(data.Relationships[args[1]]); err != nil {
return err
}
if err := json.NewDecoder(buf).Decode(relationship); err != nil {
return err
}
Copy link

Choose a reason for hiding this comment

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

Question: Can you check if the same pattern is happening for other error scenarios ?

I saw at some places, we are setting the value of er and letting the flow either break or continue

Copy link
Author

Choose a reason for hiding this comment

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

followed the same pattern on suggested areas.

Choose a reason for hiding this comment

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

please add a break for all newly added errors

Copy link
Author

Choose a reason for hiding this comment

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

added break to newly added errors


data := relationship.Data

Expand Down Expand Up @@ -483,7 +487,9 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node)

buf := bytes.NewBuffer(nil)
relDataStr := data.Relationships[args[1]]
json.NewEncoder(buf).Encode(relDataStr)
if err := json.NewEncoder(buf).Encode(relDataStr); err != nil {
return err
}

isExplicitNull := false
relationshipDecodeErr := json.NewDecoder(buf).Decode(relationship)
Expand Down
49 changes: 37 additions & 12 deletions request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"io"
"log"
"reflect"
"sort"
"strconv"
Expand Down Expand Up @@ -405,7 +406,9 @@ func TestUnmarshalNullableRelationshipsNonNullValue(t *testing.T) {
}

outBuf := bytes.NewBuffer(nil)
json.NewEncoder(outBuf).Encode(payload)
if err := json.NewEncoder(outBuf).Encode(payload); err != nil {
t.Fatalf("failed to encode: %v", err)
}

out := new(WithNullableAttrs)

Expand Down Expand Up @@ -442,7 +445,9 @@ func TestUnmarshalNullableRelationshipsExplicitNullValue(t *testing.T) {
}

outBuf := bytes.NewBuffer(nil)
json.NewEncoder(outBuf).Encode(payload)
if err := json.NewEncoder(outBuf).Encode(payload); err != nil {
t.Fatalf("failed to encode: %v", err)
}

out := new(WithNullableAttrs)

Expand All @@ -467,7 +472,9 @@ func TestUnmarshalNullableRelationshipsNonExistentValue(t *testing.T) {
}

outBuf := bytes.NewBuffer(nil)
json.NewEncoder(outBuf).Encode(payload)
if err := json.NewEncoder(outBuf).Encode(payload); err != nil {
log.Fatalf("failed to encode: %v", err)
}

out := new(WithNullableAttrs)

Expand All @@ -490,7 +497,9 @@ func TestUnmarshalNullableRelationshipsNoRelationships(t *testing.T) {
}

outBuf := bytes.NewBuffer(nil)
json.NewEncoder(outBuf).Encode(payload)
if err := json.NewEncoder(outBuf).Encode(payload); err != nil {
t.Fatalf("failed to encode: %v", err)
}

out := new(WithNullableAttrs)

Expand Down Expand Up @@ -1621,7 +1630,9 @@ func samplePayload() io.Reader {
}

out := bytes.NewBuffer(nil)
json.NewEncoder(out).Encode(payload)
if err := json.NewEncoder(out).Encode(payload); err != nil {
log.Printf("failed to encode: %v", err)
}

return out
}
Expand All @@ -1639,7 +1650,9 @@ func samplePayloadWithID() io.Reader {
}

out := bytes.NewBuffer(nil)
json.NewEncoder(out).Encode(payload)
if err := json.NewEncoder(out).Encode(payload); err != nil {
log.Printf("failed to encode: %v", err)
}

return out
}
Expand All @@ -1654,7 +1667,9 @@ func samplePayloadWithBadTypes(m map[string]interface{}) io.Reader {
}

out := bytes.NewBuffer(nil)
json.NewEncoder(out).Encode(payload)
if err := json.NewEncoder(out).Encode(payload); err != nil {
log.Printf("failed to encode: %v", err)
}

return out
}
Expand All @@ -1669,7 +1684,9 @@ func sampleWithPointerPayload(m map[string]interface{}) io.Reader {
}

out := bytes.NewBuffer(nil)
json.NewEncoder(out).Encode(payload)
if err := json.NewEncoder(out).Encode(payload); err != nil {
log.Printf("failed to encode: %v", err)
}

return out
}
Expand All @@ -1684,7 +1701,9 @@ func samplePayloadWithNullableAttrs(m map[string]interface{}) io.Reader {
}

out := bytes.NewBuffer(nil)
json.NewEncoder(out).Encode(payload)
if err := json.NewEncoder(out).Encode(payload); err != nil {
log.Printf("failed to encode: %v", err)
}

return out
}
Expand Down Expand Up @@ -1761,17 +1780,23 @@ func samplePayloadWithSideloaded() io.Reader {
testModel := testModel()

out := bytes.NewBuffer(nil)
MarshalPayload(out, testModel)
if err := MarshalPayload(out, testModel); err != nil {
log.Printf("failed to marshal payload: %v", err)
}

return out
}

func sampleSerializedEmbeddedTestModel() *Blog {
out := bytes.NewBuffer(nil)
MarshalOnePayloadEmbedded(out, testModel())
if err := MarshalOnePayloadEmbedded(out, testModel()); err != nil {
log.Printf("failed to marshal one payload embedded: %v", err)
}

blog := new(Blog)
UnmarshalPayload(out, blog)
if err := UnmarshalPayload(out, blog); err != nil {
log.Printf("failed to unmarshal payload: %v", err)
}

return blog
}
Expand Down
24 changes: 18 additions & 6 deletions response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ func TestMarshalPayload(t *testing.T) {

// One
out1 := bytes.NewBuffer(nil)
MarshalPayload(out1, book)
if err := MarshalPayload(out1, book); err != nil {
t.Fatal(err)
}

if err := json.Unmarshal(out1.Bytes(), &jsonData); err != nil {
t.Fatal(err)
Expand All @@ -29,7 +31,9 @@ func TestMarshalPayload(t *testing.T) {

// Many
out2 := bytes.NewBuffer(nil)
MarshalPayload(out2, books)
if err := MarshalPayload(out2, books); err != nil {
t.Fatal(err)
}

if err := json.Unmarshal(out2.Bytes(), &jsonData); err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -936,7 +940,9 @@ func TestMarshal_Times(t *testing.T) {
}
// Use the standard JSON library to traverse the genereated JSON payload.
data := map[string]interface{}{}
json.Unmarshal(out.Bytes(), &data)
if err := json.Unmarshal(out.Bytes(), &data); err != nil {
t.Fatal(err)
}
if tc.verification != nil {
if err := tc.verification(data); err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -1017,7 +1023,9 @@ func TestNullableRelationship(t *testing.T) {

// Use the standard JSON library to traverse the genereated JSON payload.
data := map[string]interface{}{}
json.Unmarshal(out.Bytes(), &data)
if err := json.Unmarshal(out.Bytes(), &data); err != nil {
t.Fatal(err)
}
if tc.verification != nil {
if err := tc.verification(data); err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -1114,7 +1122,9 @@ func TestNullableAttr_Time(t *testing.T) {
}
// Use the standard JSON library to traverse the genereated JSON payload.
data := map[string]interface{}{}
json.Unmarshal(out.Bytes(), &data)
if err := json.Unmarshal(out.Bytes(), &data); err != nil {
t.Fatal(err)
}
if tc.verification != nil {
if err := tc.verification(data); err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -1184,7 +1194,9 @@ func TestNullableAttr_Bool(t *testing.T) {
}
// Use the standard JSON library to traverse the genereated JSON payload.
data := map[string]interface{}{}
json.Unmarshal(out.Bytes(), &data)
if err := json.Unmarshal(out.Bytes(), &data); err != nil {
t.Fatal(err)
}
if tc.verification != nil {
if err := tc.verification(data); err != nil {
t.Fatal(err)
Expand Down
5 changes: 4 additions & 1 deletion runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,13 @@ func (r *Runtime) UnmarshalPayload(reader io.Reader, model interface{}) error {

// UnmarshalManyPayload has docs in request.go for UnmarshalManyPayload.
func (r *Runtime) UnmarshalManyPayload(reader io.Reader, kind reflect.Type) (elems []interface{}, err error) {
r.instrumentCall(UnmarshalStart, UnmarshalStop, func() error {
err = r.instrumentCall(UnmarshalStart, UnmarshalStop, func() error {
elems, err = UnmarshalManyPayload(reader, kind)
return err
})
if err != nil {
return nil, err
}

return
}
Expand Down
Loading