Wikodit / js-data-jsonapi-light

JSData adapter which serialize and deserialize JSONApi response and requests.
MIT License
8 stars 4 forks source link

hypen cased attributes #32

Closed phortx closed 6 years ago

phortx commented 6 years ago

Hi,

nice work! I was looking for this adapter for a long time :)

The JSON/REST API Specification also supports hyphen-cased keys in the attributes field. My API delivers hyphen-cased attributes keys and I have a tough time getting this running.

I tried to write a beforeDeserialize method, but due to the included field, there can be many attributes fields deep nested anywhere in the response. So it's hard to convert them all.

Do you have an idea how to solve this? Is there a method which is called for each attribute, which I could overload or something?

However a configuration flag for this would be awesome.

Thanks for your time!

phortx commented 6 years ago

I found another issue: The specification says that the type fields are lowercase and plural, while js-data-jsonapi-light expectes them to named the same as the mappers are named.

I fixed both for me by writing a rather complex beforeDeserialize and beforeSerialize method but I think this should be part of this adapter.

Tronix117 commented 6 years ago

Hi @phortx, Could you link me where it says type should be lowercase and plural ? The only thing I can find in the documentation

The values of type members MUST adhere to the same constraints as member names.

Note: This spec is agnostic about inflection rules, so the value of type can be either plural or singular. However, the same value should be used consistently throughout an implementation.

And:

All member names used in a JSON API document MUST be treated as case sensitive by clients and servers, and they MUST meet all of the following conditions:

Member names MUST contain at least one character.
Member names MUST contain only the allowed characters listed below.
Member names MUST start and end with a “globally allowed character”, as defined below.

To enable an easy mapping of member names to URLs, it is RECOMMENDED that member names use only non-reserved, URL safe characters specified in RFC 3986.

The reason behind my logic doing so, is that depending on the API, the type value will be inconsistent, so I was just making sure it was consistent with the mapper. However maybe we could add an option along the mapper: jsdataTypeName which accept either a function (to be put in your defaults) or a string. This value will then be used to map the server response.


For your first issue, I don't really understand, it should work, a key is a key and it may contains all special characters you want, I do not filter anything voluntarily. Could you detail a bit more your issue with some plunker or examples ?

phortx commented 6 years ago

Thank you for your reply!

You're absolutly right. I took it from the examples. The examples are with hyphen-cased and with singular types. Seems this isn't specified :)

However this increases the need to configure that even more.

option along the mapper: jsdataTypeName which accept either a function (to be put in your defaults) or a string.

Sounds pretty nice :)

Could you detail a bit more your issue with some plunker or examples ?

I think, I should add that the properties in my mapper schema are all camelCased (which absolute makes sense in javascript), but in my API they are dashed (I use a lib (Trailblazer Roar) to generate the API, which outputs hyphen-cased keys.

Tronix117 commented 6 years ago

Ok, for now, for the resource name, I'm using store.getMapper(type) to get the mapper, you could override this method so that it camelCase and Singularize the given type.

For the attributes, it's not really relevant to this adapter, but rather to the way JSData works, and by design, usualy you want to keep the way the attributes are, and not change their format. I think it is not a good practice to change the format of the keys. When you read the API documentation as a new developer on your project, you expect the key to be my_key, so you write that, but because the implementation convert it to CamelCase, you should have written myKey.

In javascript, it's just a standard to have camelCase, in absolute, snakeCase is also totaly fine. But here it's some properties that are coming from an external datasource, not really JavaScript properties, so my advice is to keep them as they are.


However if you absolutely want to convert them, you should do it in the afterDeserialize and in the beforeSerialize.

You want to do it as close to JSData as possible, this way you will only have a simple structure to deal with.

mapper.save -> jsData -> beforeSerialize -> jsDataJsonApiLight -> afterSerialize
mapper.find -> beforeDeserialize -> jsDataJsonApiLight -> afterDeserialize -> jsData

I should not add an option for this, because it is something that should be entirely handled by JSData, not by the adapter.

phortx commented 6 years ago

usualy you want to keep the way the attributes are, and not change their format.

I think usually I want to have as few work to do as possible. The Lib which generates my API produces pluralized types and hyphen-cased attributes. In JS I want to work with camelCased attributes, because post.categoryName is much more readable then post['category-name'] The lib which consumes my API (js-data-jsonapi-light) just passes what comes from the API to js-data. So I have to invest work in one of the both libs. Either server side oder client side. The best way to avoid this work is to add a configuration param to one of them. Due to the fact that it could be, that I'm using this adapter in a project where I don't have any influence on the API (like consuming a third party api or so), I think the adapter should have an configuration option to define how to handle attribute names and type values.

However if you absolutely want to convert them, you should do it in the afterDeserialize and in the beforeSerialize.

I understand your argument and you're right, however I had to implement it in beforeDeserialize and afterSerialize out of reasons I forgot (something with relations I think). It works now for me, thank you for your help :)

Nevertheless I think a config option would make sense.

Tronix117 commented 6 years ago

That is weird you add to implement it in those, I'd like to know why you couldn't do that in the afterDeserialize, maybe it's a bug I can fix ?

For the configuration option, I understand your concern, but it should be handled in jsdata-http-adapter, because it's not something specific to jsonAPI, maybe someone using another API have the same issue. I guess that is why we provide hooks like after/before serialize/deserialize, to answer needs like this one.

phortx commented 6 years ago

Ehm I think it was because of the input for afterDerserialize vs beforeDeserialize. For after, the data param only contains the attributes while for before the data param contains the whole response (including other fields like relationships and included and so on). And I needed all of them (to transform all fields) otherwise relationships were broken. But not sure anymore, sorry.

but it should be handled in jsdata-http-adapter

Yes, that would make sense.

Tronix117 commented 6 years ago

Yep, included stuff should also goes through the afterDeserialize if I did everything correctly ;)