absinthe-graphql / absinthe

The GraphQL toolkit for Elixir
http://absinthe-graphql.org
Other
4.29k stars 527 forks source link

Conversion between snake and camel case inconsistencies #601

Open kevmojay opened 6 years ago

kevmojay commented 6 years ago

I ran into an issue where the following field field(:v_i_n, :string) would produce the graphql field "vIN"

When requesting vIN, it would produce an error "errors":[{"message":"Cannot query field \"vIN\" on type \"Metadata\". Did you mean \"vIN\"?","locations":[{"line":30,"column":0}]}]}

This is because absinthe uses Macro.camelize and Macro.underscore. These aren't necessarily reverse operations.

iex(19)> "v_i_n" |> Macro.camelize |> Macro.underscore
"vin"
iex(20)> "foo_bar" |> Macro.camelize |> Macro.underscore
"foo_bar"

I was considering creating a PR to create a more consistent conversion but figured it would be too much of a breaking change.

I was curious if there were any potential options of how this should be solved, if at all.

ku1ik commented 6 years ago

I just ran into similar problem. We have field :adress_line_1 in input object schema, and when addressLine1 is included we get Unknown field error.

iex(1)> "address_line_1" |> Macro.camelize |> Macro.underscore
"address_line1"

Sending value for this field under addressLine_1 works, but this is not what GQL schema shows in GraphiQL (it underlines it as error).

alejandrodnm commented 6 years ago

I was going crazy I have a percent_change_7d field, the unittests were passing because they are in snake_case, but graphiql complained with

 "Cannot query field \"percentChange7d\" on type \"Quote\". Did you mean \"percentChange24h\", \"percentChange1h\", or \"percentChange7d\"?",

The weird part is that Graphiql autocompletes the correct name.

screen shot 2018-09-03 at 00 39 00

Changing it to percentChange_7d like @sickill suggested makes the query work.

ku1ik commented 6 years ago

I think that quite safe way of fixing that is to keep %{Macro.camelize(field_1_name) => field_1_name, ...} map on the schema, and use it for lookup instead of relying on Macro.underscore. If that sounds like acceptable solution I'd be happy to send PR.

benwilson512 commented 6 years ago

Hey folks,

Thanks for offering a PR! This is all going to be reworked as part of the Absinthe 1.5 schema update. We'll be providing some docs on what to do in situations like this. The name solution is a good one for now.

On Tue, Sep 4, 2018 at 6:28 AM Marcin Kulik notifications@github.com wrote:

I think that quite safe way of fixing that is to keep %{Macro.camelize(field_1_name) => field_1_name, ...} map on the schema, and use it for lookup instead of relying on Macro.underscore. If that sounds like acceptable solution I'd be happy send PR.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/absinthe-graphql/absinthe/issues/601#issuecomment-418366980, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPICWevXXgy_TPW1h4PE9hwSu8Y-jt1ks5uXoAIgaJpZM4WOgsC .

alanpeabody commented 5 years ago

Hi @benwilson512 I wanted to bring this one back up and see if there is anything I can do to help. With a little direction I am happy to open a PR.

I pulled down 1.5.0-alpha.2 but it appears to suffer from the same problem.

This is a pretty big blocker for me when working with api clients that generate queries automatically from introspection.

I would prefer to help fix it in absinthe rather then come up with new field names with out numbers or something.

benwilson512 commented 5 years ago

@alanpeabody is this a blocker there? Adapters should provide everything you need there. There are some consistency issues when trying to use the built in adapter with specific field names (involving numbers for example) but even those are handleable with adjusters.

alanpeabody commented 5 years ago

Thanks for directing me to look at and Adapter. I have solved for now with a custom adapter delegating most processing to the LanguageConventions Adapter:

  @impl true
  def to_internal_name(external_name, role) do
    external_name
    |> String.replace(~r/([a-zA-Z])(\d+)/, "\\1_\\2")
    |> LanguageConventions.to_internal_name(role)
  end

However, to me, this isn't an ideal solution for something that seems like it should work out of the box.

For reference my schema is something like:

  object :ticker do
    # ...
    field :price_change_24h, :currency_price
    field :price_change_24h_pct, :float
    field :high_price_24h, :currency_price
    field :low_price_24h, :currency_price
    field :volume_24h, :currency_amount
    # ...
  end

And a query generated interactively with GraphiQL using the introspective API is:

query {
  getTicker() {
    highPrice24h { amount }
    lowPrice24h { amount }
    priceChange24h { amount }
    priceChange24hPct
    volume24h { amount }
  }
}

However, those two combined generate this error:

{
      "message": "Cannot query field \"highPrice24h\" on type \"Ticker\". Did you mean \"priceChange24hPct\", \"priceChange24h\", \"lowPrice24h\", or \"highPrice24h\"?"
}

The fact that the query is passing in what is the correct field, the error says "this field is incorrect" and then lists the exact field you input isn't a great experience for people trying to develop against and API using the provided documentation.

Using a custom adapter works for me, but I do think the correct fix would be in Absinthe itself, and I am happy to help with it.

iautom8things commented 5 years ago

Glad I found this. I'm just getting started with GraphQL and Absinthe. I'm working with a platform that I do not have control over the naming of tables/columns and there's prolific use of underscores, and most notably double underscores.

I thought I was going crazy when I was getting the error Cannot query field "foo" ... Did you mean "foo"?.

I'll try to take a look at Adapters like was suggested. There was a mention that v1.5 would make some changes that address this. Are those still in the works?

Thank you.

benwilson512 commented 5 years ago

@iautom8things keep in mind that nothing requires that you use the same graphql field names as your database column names. In fact you can even rename them at the ecto layer to be more ergonomic.

jfrolich commented 5 years ago

Would be great if it can be resolved in 1.5! Using a custom adapter now, but not the best solution.

jfrolich commented 5 years ago

It might be better to have a slightly different underscore function by default, that puts an underscore before digits by default (looks like a lot of people are running into this). I could pull-request this.

nishant-ghodke commented 4 years ago

I had the same issue. Columns names in MySQL table, and properties in Ecto model and GraphQL schema were in camelCase.

I fixed the issue by updating the case of all properties and columns to snake_case.

Absinthe makes the model properties accessible in camelCase but expects the same to be declared in snake_case.

Absinthe version: 1.4.16

thebigredgeek commented 4 years ago

This is still an issue in 1.5

dotdotdotpaul commented 2 years ago

Still a problem in 2022. Even worse, I defined an input field with the name :fooBar (actual name hidden for proprietary reasons), and when I ran a test giving it that key, the error it generated was this:

In field \\\"fooBar\\\": Unknown field. Did you mean \\\"fooBar\\\"?

Yeah, I almost had to go buy a new laptop after I got that gem on my screen.