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

Usage of an external package to simplify interactions with the JSON API document #2

Open
nstratos opened this issue Feb 26, 2017 · 0 comments

Comments

@nstratos
Copy link
Owner

nstratos commented Feb 26, 2017

This issue highlights the benefits and drawbacks of using an external package to interact with the JSON API document, especially the more complex case of unmarshaling to structs. The aim is to have a clear overview, discuss and conclude what is going to be used for the development of this package.

Candidates

Benefits of adding an external dependency

  1. Simplify the code needed for (un)marshaling to structs with unmarshal of included relationships being the most complex case.
  2. Having a robust and battle-tested codebase that handles the intricacies of JSON API for us.
  3. Be resistant to change. This is especially important since the Kitsu API is not yet stable.

Drawbacks

  1. Having a vendored dependency on a package that the user has to import is known to be problematic. This can be mitigated by making sure no code from the dependency is leaked to the final user. There is also heavy development of the dep tool. While it is not yet complete it can help with this problem.
  2. Some of the code gets uglier especially since some packages like google/jsonapi use reflection but the benefit is that the structs of go-kitsu become much simpler and prettier.
  3. The standard cost of dependencies. Obviously having zero dependencies is much more preferred especially for a package like this. Nevertheless it seems that just the case of unmarshaling included relationships is complex enough to justify the cost.

Comparison of candidates

After trying other packages for a while now, it seems that the most robust ones are google/jsonapi and manyminds/api2go. What api2go does better is that it more or less avoids reflection and uses interfaces to let the user control marshaling. Unfortunately at the time of writing, the client capabilities of api2go aka unmarshaling are not yet complete (manyminds/api2go#112).

Other packages like smotes/jsonapi and gonfire/jsonapi while they might help with 2. and 3. they do not help with 1. as filling our structs with data has to be done manually. Nevertheless they might prove more performant overall since they avoid reflection.

That said, since we are dealing with fetching data from an API the overhead of reflection shouldn't be a problem which should make google/jsonapi the strongest candidate as it helps with 1. by allowing to easily unmarshal to structs using custom tags like jsonapi:"attr". Additionaly at the time of writing google/jsonapi seems to be more actively maintained than the rest.

Known Issues with google/jsonapi and possible solutions

While google/jsonapi might be the strongest candidate it comes with a few issues:

  • Right now there seems to be no way of having access to JSON API pagination links without parsing the document two times (Pagination access google/jsonapi#64). This can be solved by modifying the function UnmarshalManyPayload to also return the map of links but obviously modifying vendored code is not the most elegant solution. The reason this is needed is for providing the user of the go-kitsu package an easy way to access pagination links when they do List requests. The idea was to use a custom Response type that holds the offset as it is done in go-github but maybe there is a better way.

  • The package seems to be unable to unmarshal attributes that are objects (Unmarshal of object/struct attributes fails google/jsonapi#74). There is already a case where this is trouble for go-kitsu when trying to get a list of anime including relationships castings.character and castings.person. castings.character includes an attribute called image which is an object containing strings like original. Since this is a field of a relationship, it is not such a big deal but there can be many more cases like this one. The simplest thing to do for now is to not include such problematic fields in our structs and hopefully the issue will get fixed eventually.

Giving google/jsonapi a test drive

Altered code which uses google/jsonapi as a vendored dependency will be committed on a separate branch to have a clearer view of how it works in practice.

Discussion and opinions are more than welcome.

nstratos added a commit that referenced this issue Mar 24, 2017
This commit replaces the usage of google/jsonapi with a fork. The fork
adds an extra function (a copy of UnmarshalManyPayload) which returns
the pagination links of JSON API documents as this functionality is not
provided by google/jsonapi. The need for the links is described in:
#2

As a side effect of using the fork, it now seems that there is no longer
a need for vendoring or using a dependency management tool so the vendor
folder, the lock.json and manifest.json files (created by the dep tool)
are deleted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 👀 In review
Development

No branches or pull requests

1 participant