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

Add option to exclude keys from underscore/dash treatment? #132

Closed
wasnotrice opened this issue Sep 14, 2018 · 11 comments
Closed

Add option to exclude keys from underscore/dash treatment? #132

wasnotrice opened this issue Sep 14, 2018 · 11 comments

Comments

@wasnotrice
Copy link
Contributor

I'm working with an API where I have a field data_schema that is an arbitrary JSON object. The API uses the dashed key format, so it shows up in requests/responses as data-schema. This is all fine until someone includes an underscore in one of their JSON object keys, such as __finalized__. So my incoming data

%{
  "data" => %{
    "attributes" => %{
      "data-schema" => %{
        "properties" => %{
          "__finalized__" => %{"title" => "Finalized", "type" => "boolean"},
        },
        "type" => "object"
      },
    },
    "relationships" => %{},
    "type" => "schemas"
  }
}

gets transformed into

%{
  "data" => %{
    "attributes" => %{
      "data-schema" => %{
        "properties" => %{
          "--finalized--" => %{"title" => "Finalized", "type" => "boolean"}
        },
        "type" => "object"
      },
      "inserted-at" => "2018-09-14T21:03:25.829318",
      "updated-at" => "2018-09-14T21:03:25.829328"
    },
    "id" => "27304",
    "relationships" => %{},
    "type" => "schemas"
  },
  "included" => [],
  "meta" => nil
}

(this is surprising to the people who used the leading underscores to indicate "don't mess with this") Ideally, the value of data-schema would not be touched at all. I'm not sure how we could accomplish that simply. But I wonder if it makes sense to add an underscore option that specifies field prefixes or patterns that would not be underscored/dashed? Any thoughts?

@doomspork
Copy link
Member

@wasnotrice does underscore_to_dash: false not do the trick here? After a quick glance through the code I believe the only time we change anything is if this is set to true

@wasnotrice
Copy link
Contributor Author

@doomspork if the api used underscored keys, then yes, it would. The problem is that the api's response keys are dashed, but one of my values data-schema is an arbitrary JSON object. Since the keys of this object are user-created values, I don't want them to be modified.

Does that make more sense?

@doomspork
Copy link
Member

@wasnotrice I don't think I'm following sorry 😞 The underscore_to_dash: false doesn't convert - -> _, it just doesn't apply the transformation the other way. If you don't need jsonapi to change anything for you, setting this to false should satisfy your requirement.

If your API is already giving you dashes and you're returning dashes without the help of `

@wasnotrice
Copy link
Contributor Author

@doomspork sorry I don't think I'm doing a very good job of explaining :)

The api is generated with this library, so the reason that the keys are dashed in the first place is that I have underscore_to_dash: true. That is desired.

But given that my api serves responses with dashed keys, when some of my fields are arbitrary JSON, I don't want the library to descend into that arbitrary JSON and dash the keys there, too. I want to say something like "transform keys to dashes in this map, but skip the value of data_schema"

@doomspork
Copy link
Member

Ah ha! I follow now. I'm not 💯on how best to accomplish this though 🤔

Maybe we could support 4 options for underscore_to_dash: true, false, only: [] and except: [], with the latter two implying true.

Something like:

underscore_to_dash:
  except: [:__finalized__]

Maybe only: [] isn't necessary?

@wasnotrice
Copy link
Contributor Author

Thanks for sticking with me :)

Sure, I think except: [:__finalized__] could work. As I've thought about it more, it might also make sense to be able to specify this list per-view rather than in the application env.

The other potential confusion is whether the :except list is the keys to skip, or the values to skip.

I'll tinker with it. It'll be easier to talk about with an implementation and test.

@doomspork
Copy link
Member

Sounds good to me @wasnotrice, a PR would be a great place to start 👍

@snewcomer
Copy link
Contributor

@wasnotrice I think except && only would be good ideas! Would it be also possible to improve this line?

String.replace("wat__foo", ~r/([a-zA-Z0-9])_([a-zA-Z0-9])/, "\\1-\\2")

Lmk what you think!

@protestContest
Copy link
Contributor

Hello! I just came across this issue, and it's a problem that applies to me also. I'm maintaining an API where I'd like the field keys to be dasherized, but some attribute values are arbitrary JSON objects whose keys I'd like to not be dasherized.

The spec says in section 7.2.2.1 "Attributes":

Attributes may contain any valid JSON value, including complex data structures involving JSON objects and arrays.

To me, this seems to make a separation between the format of the JSON:API document and the values of attributes, whose format is arbitrary. I think it would make sense for a config option that transforms field keys, but doesn't change attribute values. This would let library users benefit from the field transformation config, but still have control over the format of the attribute values.

Any thoughts on this approach? I'm happy to work on an implementation if it makes sense.

@mattpolzin
Copy link
Member

mattpolzin commented Feb 6, 2024

@protestContest I am at least initially liking your idea. To me, it's nice if the options for "dash conversion" are fairly high level (i.e. operate on everything or operate on everything except attributes) because we do all have the option to turn the feature off and do our own transformation outside of this library. In other words, I think trying to support increasingly niche controls reaches diminishing returns from the perspective of maintaining this library.

I'd be interested in reviewing a PR from you that allowed for excluding everything nested within attributes from transformation.

@mattpolzin
Copy link
Member

I'll call this issue closed by #310 but if anyone wants to have it re-opened (or possibly even better open a new targeted ticket) because your needs aren't covered by what's available so far, please do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants