Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unmarshal Links type to 'links' annotated struct fields #10

Merged
merged 2 commits into from
May 18, 2021

Conversation

chrisarcand
Copy link
Member

@chrisarcand chrisarcand commented May 14, 2021

These changes add the ability to unmarshal links members to links-annotated struct fields.

The specification for links members can be found here: https://jsonapi.org/format/#document-links

Note that this does not change the existing marshaling behavior of links (the Linkable interface methods), though a fully fleshed out implementation probably should - it's awkward that now interface methods are used to marshal but struct fields for unmarshaling.

In other words, an implementation of links marshaling similar to the implementation proposed in PR 58 of the canonical google/jsonapi repository would be most appropriate to pair with these unmarshaling changes. The actual implementation that was accepted is PR 57, utilizing those Linkable/Metable interfaces.

From 57:

After looking at both implementations, I think I'd like to vote for this one because it feels flexible, and doesn't add any additional fields to our structs. In general I don't think LINK and META objects really have much to do with the resource objects themselves, they are really more related to the context in which the object is used. This approach keeps structs cleaner.

This is somewhat incorrect, and assumes that this package is always used in the context of a server and not a client (which appears to be the case in general for google/jsonapi). In a client, links members in responses can contain information that is vital to the functionality of the resource itself, so we add the ability to fetch that information here (and leave the weirdness of marshaling using a different mechanism for another day).

Test Output
» go test -v
=== RUN   TestErrorObjectWritesExpectedErrorMessage
--- PASS: TestErrorObjectWritesExpectedErrorMessage (0.00s)
=== RUN   TestMarshalErrorsWritesTheExpectedPayload
=== RUN   TestMarshalErrorsWritesTheExpectedPayload/TestFieldsAreSerializedAsNeeded
=== RUN   TestMarshalErrorsWritesTheExpectedPayload/TestMetaFieldIsSerializedProperly
--- PASS: TestMarshalErrorsWritesTheExpectedPayload (0.00s)
    --- PASS: TestMarshalErrorsWritesTheExpectedPayload/TestFieldsAreSerializedAsNeeded (0.00s)
    --- PASS: TestMarshalErrorsWritesTheExpectedPayload/TestMetaFieldIsSerializedProperly (0.00s)
=== RUN   TestUnmarshall_attrStringSlice
--- PASS: TestUnmarshall_attrStringSlice (0.00s)
=== RUN   TestUnmarshalToStructWithPointerAttr
--- PASS: TestUnmarshalToStructWithPointerAttr (0.00s)
=== RUN   TestUnmarshalPayload_missingTypeFieldShouldError
--- PASS: TestUnmarshalPayload_missingTypeFieldShouldError (0.00s)
=== RUN   TestUnmarshalPayload_ptrsAllNil
--- PASS: TestUnmarshalPayload_ptrsAllNil (0.00s)
=== RUN   TestUnmarshalPayloadWithPointerID
--- PASS: TestUnmarshalPayloadWithPointerID (0.00s)
=== RUN   TestUnmarshalPayloadWithPointerAttr_AbsentVal
--- PASS: TestUnmarshalPayloadWithPointerAttr_AbsentVal (0.00s)
=== RUN   TestUnmarshalToStructWithPointerAttr_BadType_bool
--- PASS: TestUnmarshalToStructWithPointerAttr_BadType_bool (0.00s)
=== RUN   TestUnmarshalToStructWithPointerAttr_BadType_MapPtr
--- PASS: TestUnmarshalToStructWithPointerAttr_BadType_MapPtr (0.00s)
=== RUN   TestUnmarshalToStructWithPointerAttr_BadType_Struct
--- PASS: TestUnmarshalToStructWithPointerAttr_BadType_Struct (0.00s)
=== RUN   TestUnmarshalToStructWithPointerAttr_BadType_IntSlice
--- PASS: TestUnmarshalToStructWithPointerAttr_BadType_IntSlice (0.00s)
=== RUN   TestStringPointerField
--- PASS: TestStringPointerField (0.00s)
=== RUN   TestMalformedTag
--- PASS: TestMalformedTag (0.00s)
=== RUN   TestUnmarshalInvalidJSON
--- PASS: TestUnmarshalInvalidJSON (0.00s)
=== RUN   TestUnmarshalInvalidJSON_BadType
=== RUN   TestUnmarshalInvalidJSON_BadType/Test_string_field
=== RUN   TestUnmarshalInvalidJSON_BadType/Test_float_field
=== RUN   TestUnmarshalInvalidJSON_BadType/Test_time_field
=== RUN   TestUnmarshalInvalidJSON_BadType/Test_time_ptr_field
--- PASS: TestUnmarshalInvalidJSON_BadType (0.00s)
    --- PASS: TestUnmarshalInvalidJSON_BadType/Test_string_field (0.00s)
    --- PASS: TestUnmarshalInvalidJSON_BadType/Test_float_field (0.00s)
    --- PASS: TestUnmarshalInvalidJSON_BadType/Test_time_field (0.00s)
    --- PASS: TestUnmarshalInvalidJSON_BadType/Test_time_ptr_field (0.00s)
=== RUN   TestUnmarshalSetsID
--- PASS: TestUnmarshalSetsID (0.00s)
=== RUN   TestUnmarshal_nonNumericID
--- PASS: TestUnmarshal_nonNumericID (0.00s)
=== RUN   TestUnmarshalSetsAttrs
--- PASS: TestUnmarshalSetsAttrs (0.00s)
=== RUN   TestUnmarshal_Times
=== RUN   TestUnmarshal_Times/default_byValue
=== RUN   TestUnmarshal_Times/default_byPointer
=== RUN   TestUnmarshal_Times/default_invalid
=== RUN   TestUnmarshal_Times/iso8601_byValue
=== RUN   TestUnmarshal_Times/iso8601_byPointer
=== RUN   TestUnmarshal_Times/iso8601_invalid
=== RUN   TestUnmarshal_Times/rfc3339_byValue
=== RUN   TestUnmarshal_Times/rfc3339_byPointer
=== RUN   TestUnmarshal_Times/rfc3339_invalid
--- PASS: TestUnmarshal_Times (0.00s)
    --- PASS: TestUnmarshal_Times/default_byValue (0.00s)
    --- PASS: TestUnmarshal_Times/default_byPointer (0.00s)
    --- PASS: TestUnmarshal_Times/default_invalid (0.00s)
    --- PASS: TestUnmarshal_Times/iso8601_byValue (0.00s)
    --- PASS: TestUnmarshal_Times/iso8601_byPointer (0.00s)
    --- PASS: TestUnmarshal_Times/iso8601_invalid (0.00s)
    --- PASS: TestUnmarshal_Times/rfc3339_byValue (0.00s)
    --- PASS: TestUnmarshal_Times/rfc3339_byPointer (0.00s)
    --- PASS: TestUnmarshal_Times/rfc3339_invalid (0.00s)
=== RUN   TestUnmarshalRelationshipsWithoutIncluded
--- PASS: TestUnmarshalRelationshipsWithoutIncluded (0.00s)
=== RUN   TestUnmarshalRelationships
--- PASS: TestUnmarshalRelationships (0.00s)
=== RUN   TestUnmarshalNullRelationship
--- PASS: TestUnmarshalNullRelationship (0.00s)
=== RUN   TestUnmarshalNullRelationshipInSlice
--- PASS: TestUnmarshalNullRelationshipInSlice (0.00s)
=== RUN   TestUnmarshalNestedRelationships
--- PASS: TestUnmarshalNestedRelationships (0.00s)
=== RUN   TestUnmarshalRelationshipsSerializedEmbedded
--- PASS: TestUnmarshalRelationshipsSerializedEmbedded (0.00s)
=== RUN   TestUnmarshalNestedRelationshipsEmbedded
--- PASS: TestUnmarshalNestedRelationshipsEmbedded (0.00s)
=== RUN   TestUnmarshalRelationshipsSideloaded
--- PASS: TestUnmarshalRelationshipsSideloaded (0.00s)
=== RUN   TestUnmarshalNestedRelationshipsSideloaded
--- PASS: TestUnmarshalNestedRelationshipsSideloaded (0.00s)
=== RUN   TestUnmarshalNestedRelationshipsEmbedded_withClientIDs
--- PASS: TestUnmarshalNestedRelationshipsEmbedded_withClientIDs (0.00s)
=== RUN   TestUnmarshalLinks
--- PASS: TestUnmarshalLinks (0.00s)
=== RUN   TestUnmarshalManyPayload
--- PASS: TestUnmarshalManyPayload (0.00s)
=== RUN   TestOnePayload_withLinks
--- PASS: TestOnePayload_withLinks (0.00s)
=== RUN   TestManyPayload_withLinks
--- PASS: TestManyPayload_withLinks (0.00s)
=== RUN   TestUnmarshalCustomTypeAttributes
--- PASS: TestUnmarshalCustomTypeAttributes (0.00s)
=== RUN   TestUnmarshalCustomTypeAttributes_ErrInvalidType
--- PASS: TestUnmarshalCustomTypeAttributes_ErrInvalidType (0.00s)
=== RUN   TestUnmarshalNestedStructPtr
--- PASS: TestUnmarshalNestedStructPtr (0.00s)
=== RUN   TestUnmarshalNestedStruct
--- PASS: TestUnmarshalNestedStruct (0.00s)
=== RUN   TestUnmarshalNestedStructSlice
--- PASS: TestUnmarshalNestedStructSlice (0.00s)
=== RUN   TestUnmarshalNestedStructPointerSlice
--- PASS: TestUnmarshalNestedStructPointerSlice (0.00s)
=== RUN   TestMarshalPayload
--- PASS: TestMarshalPayload (0.00s)
=== RUN   TestMarshalPayloadWithNulls
--- PASS: TestMarshalPayloadWithNulls (0.00s)
=== RUN   TestMarshal_attrStringSlice
--- PASS: TestMarshal_attrStringSlice (0.00s)
=== RUN   TestWithoutOmitsEmptyAnnotationOnRelation
--- PASS: TestWithoutOmitsEmptyAnnotationOnRelation (0.00s)
=== RUN   TestWithOmitsEmptyAnnotationOnRelation
--- PASS: TestWithOmitsEmptyAnnotationOnRelation (0.00s)
=== RUN   TestWithOmitsEmptyAnnotationOnRelation_MixedData
--- PASS: TestWithOmitsEmptyAnnotationOnRelation_MixedData (0.00s)
=== RUN   TestWithOmitsEmptyAnnotationOnAttribute
--- PASS: TestWithOmitsEmptyAnnotationOnAttribute (0.00s)
=== RUN   TestMarshalIDPtr
--- PASS: TestMarshalIDPtr (0.00s)
=== RUN   TestMarshalOnePayload_omitIDString
--- PASS: TestMarshalOnePayload_omitIDString (0.00s)
=== RUN   TestMarshall_invalidIDType
--- PASS: TestMarshall_invalidIDType (0.00s)
=== RUN   TestOmitsEmptyAnnotation
--- PASS: TestOmitsEmptyAnnotation (0.00s)
=== RUN   TestHasPrimaryAnnotation
--- PASS: TestHasPrimaryAnnotation (0.00s)
=== RUN   TestSupportsAttributes
--- PASS: TestSupportsAttributes (0.00s)
=== RUN   TestOmitsZeroTimes
--- PASS: TestOmitsZeroTimes (0.00s)
=== RUN   TestMarshal_Times
=== RUN   TestMarshal_Times/default_byValue
=== RUN   TestMarshal_Times/default_byPointer
=== RUN   TestMarshal_Times/iso8601_byValue
=== RUN   TestMarshal_Times/iso8601_byPointer
=== RUN   TestMarshal_Times/rfc3339_byValue
=== RUN   TestMarshal_Times/rfc3339_byPointer
--- PASS: TestMarshal_Times (0.00s)
    --- PASS: TestMarshal_Times/default_byValue (0.00s)
    --- PASS: TestMarshal_Times/default_byPointer (0.00s)
    --- PASS: TestMarshal_Times/iso8601_byValue (0.00s)
    --- PASS: TestMarshal_Times/iso8601_byPointer (0.00s)
    --- PASS: TestMarshal_Times/rfc3339_byValue (0.00s)
    --- PASS: TestMarshal_Times/rfc3339_byPointer (0.00s)
=== RUN   TestSupportsLinkable
--- PASS: TestSupportsLinkable (0.00s)
=== RUN   TestInvalidLinkable
--- PASS: TestInvalidLinkable (0.00s)
=== RUN   TestSupportsMetable
--- PASS: TestSupportsMetable (0.00s)
=== RUN   TestRelations
--- PASS: TestRelations (0.00s)
=== RUN   TestNoRelations
--- PASS: TestNoRelations (0.00s)
=== RUN   TestMarshalPayloadWithoutIncluded
--- PASS: TestMarshalPayloadWithoutIncluded (0.00s)
=== RUN   TestMarshalPayload_many
--- PASS: TestMarshalPayload_many (0.00s)
=== RUN   TestMarshalMany_WithSliceOfStructPointers
--- PASS: TestMarshalMany_WithSliceOfStructPointers (0.00s)
=== RUN   TestMarshalManyWithoutIncluded
--- PASS: TestMarshalManyWithoutIncluded (0.00s)
=== RUN   TestMarshalMany_SliceOfInterfaceAndSliceOfStructsSameJSON
--- PASS: TestMarshalMany_SliceOfInterfaceAndSliceOfStructsSameJSON (0.00s)
=== RUN   TestMarshal_InvalidIntefaceArgument
--- PASS: TestMarshal_InvalidIntefaceArgument (0.00s)
PASS
ok      github.com/hashicorp/jsonapi    0.101s

These changes add the ability to unmarshal links members to
links-annotated struct fields.

The specification for links members can be found here:
https://jsonapi.org/format/#document-links

Note that this does not change the existing marshaling behavior of
links (the Linkable interface methods), though a fully fleshed out
implementation probably should - it's awkward that now interface methods
are used to marshal but struct fields for unmarshaling.

In other words, an implementation of links marshaling similar to the
implementation proposed in PR 58 of the canonical google/jsonapi
repository would be most appropriate to pair with these unmarshaling
changes. The actual implementation that was accepted is PR 57, utilizing
those Linkable/Metable interfaces.

From 57:

> After looking at both implementations, I think I'd like to vote for
this one because it feels flexible, and doesn't add any additional
fields to our structs. In general I don't think LINK and META objects
really have much to do with the resource objects themselves, they are
really more related to the context in which the object is used. This
approach keeps structs cleaner.

This is subjectively wrong, and assumes that this package is used in the
context of a server and not a client. In a client, links members can
contain information that is vital to the functionality of the resource
itself, so we add that here.
@chrisarcand chrisarcand requested a review from cam-stitt May 14, 2021 03:41
Copy link
Member

@cam-stitt cam-stitt left a comment

Choose a reason for hiding this comment

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

Looks good! Only one small cleanup.

Co-authored-by: Cameron Stitt <[email protected]>
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.

2 participants