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

Support member name transformers #17

Open
zachdaniel opened this issue Jun 7, 2020 · 9 comments
Open

Support member name transformers #17

zachdaniel opened this issue Jun 7, 2020 · 9 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@zachdaniel
Copy link
Contributor

Currently, everything is snake_case but we want to let users configure a name transformer at the API level, to turn everything into kebab case or camelcase.

@zachdaniel zachdaniel added the enhancement New feature or request label Jun 7, 2020
@zachdaniel zachdaniel added the good first issue Good for newcomers label Sep 21, 2020
@mario-mazo
Copy link

mario-mazo commented Sep 26, 2020

Steps:
step 1
will be adding an optional configuration to this schema
https://github.com/ash-project/ash_json_api/blob/master/lib/ash_json_api/api/api.ex
called member_name_transformer

step 2
then you need to make a behaviour module called AshJsonApi.MemberNameTransformer
that behaviour should have two callbacks. I think the first one should be called transform_in(string) and the other should be transform_out(string)
This would be an example of a transformer we'd probably provide by default

defmodule AshJsonApi.MemberNameTransformer.CamelCase do
  @behaviour AshJsonAPi.MemberNameTransformer

  def transform_out(snake_case) do
    convert_snake_case_to_camelcase(snake_case)
  end

  def transform_in(camel_case) do
    convert_camel_case_to_snake_case(camel_case)
  end
end

I mean, also maybe member_name_transformer is a bad name, it could just be like...key_transformer or something, feel free to propose less confusing names

@mario-mazo
Copy link

mario-mazo commented Sep 26, 2020

@zachdaniel I added our discussion here, so maybe other people can benefit.

I will try to do this

@mario-mazo mario-mazo self-assigned this Sep 26, 2020
@zachdaniel
Copy link
Contributor Author

Good call putting the chat here 👍 The final step of this will be to use the member name transformer throughout the application. Specifically, in AshJsonApi.Request, where we parse the input, AshJsonApi.Serializer where we form our responses, and AshJsonApi.JsonSchema which is used as validation and documentation.

@mario-mazo
Copy link

I should be able to tackle this by in the next 3 weeks. If somebody can jump in soon just ping me!

@binarypaladin
Copy link

I'd like to take this one, but I have a few questions.

  1. What do I need to know to make this consistent or mostly consistent with GraphQL? I see it mostly like this, I want to be able to specifically override the name of any field and then set a default transformer/inflector. Can you give me an example of what the configuration SHOULD look like?

  2. I'd like to do the same with filters. For instance I'd love to be able to change greater_than_or_equal to gte. The URLs in JSON:API can already get pretty unwieldy.

  3. I worked with an API recently that actually allowed the client to choose the inflection type in a header and configure a default. I thought it was pretty cool mainly because: I actually prefer kebab case in the API because it feels the most language neutral to me. It annoys me that the "standard" changed to camel case in 1.1, but I understand why: to make JS consumption easier. Why not just let the client choose if you can support more than one?

Are transformations computed entirely at compile time? I ask because in my own implementation I created maps at compile time so I wasn't transforming on the fly all the time. It would definitely complicate #3 a bit.

@zachdaniel
Copy link
Contributor Author

What do I need to know to make this consistent or mostly consistent with GraphQL? I see it mostly like this, I want to be able to specifically override the name of any field and then set a default transformer/inflector. Can you give me an example of what the configuration SHOULD look like?

There are two different things here. AshGraphql does not support a member name transformer. Everything is always camelcase in the api. What AshGraphql has is a tool for remapping field names. What that would look like is a set of configurations like argument_names [foo: :bar] on routes, and field_names [foo: :bar] on the resource etc. However, at these levels IMO we should always be dealing with snake case.

The concept of a "member name transformer" would be used for making wholesale transformations of the entire API. Ignoring performance, it would look like everything in the serializer, deserializer, and json schema builder doing something like MemberNameTransformer.transform(name, field).

Ultimately there is some non-trivial things to consider here. For example, inside of an attribute whose type is just :map, should we remap the contents arbitrarily? Do a deep traversal? (probably not). Likely the answer is to just leave them alone, but we'll have to think about these things :)

I'd like to do the same with filters. For instance I'd love to be able to change greater_than_or_equal to gte. The URLs in JSON:API can already get pretty unwieldy.
I think gte may already be accepted, but yes we can support some kind of filter handler where you get filter objects and the opportunity to transform them ahead of time.

I worked with an API recently that actually allowed the client to choose the inflection type in a header and configure a default. I thought it was pretty cool mainly because: I actually prefer kebab case in the API because it feels the most language neutral to me. It annoys me that the "standard" changed to camel case in 1.1, but I understand why: to make JS consumption easier. Why not just let the client choose if you can support more than one?

For this reason, I think we should do this all dynamically (with perhaps the exception of member names that appear literally in ash_json_api). Would be great to support this use case, and if we can kill two birds with one stone then all the better. (We'd of course make supporting this header opt-in)

@binarypaladin
Copy link

There are two different things here. AshGraphql does not support a member name transformer. Everything is always camelcase in the api. What AshGraphql has is a tool for remapping field names. What that would look like is a set of configurations like argument_names [foo: :bar] on routes, and field_names [foo: :bar] on the resource etc. However, at these levels IMO we should always be dealing with snake case.

Right. Transformations need to happen at the last possible second and be relevant only the serialization/deserialization layer.

Ultimately there is some non-trivial things to consider here. For example, inside of an attribute whose type is just :map, should we remap the contents arbitrarily? Do a deep traversal? (probably not). Likely the answer is to just leave them alone, but we'll have to think about these things :)

In my own API, where we use camelcase but everything else under the hood are just Ecto schemas using snake case, fields that are maps come in two flavors: plain old maps (like meta data) and nested structs which are mapped. What I do in that case is use an embedded struct and apply my mappings to the embedded resource.

I did look at the JSON:API code around Christmas when I was toying with this and was just like, "Well, I would have done this if I'd been given two weeks off rather than a major deadline." Haha.

I think the transformer is going to need to take some kind of either options or context as in addition to the resource and name.

@zachdaniel
Copy link
Contributor Author

I don't think we should provide the resource or any context at all to the transformer actually. It should be all or nothing. If you use an embedded resource, then we transform its member names. Then nested maps are just "your own problem" to handle mixed casing if you want.

@zachdaniel
Copy link
Contributor Author

So it really should just be two functions that take strings and should be inverses of eachother.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants