jsonapi icon indicating copy to clipboard operation
jsonapi copied to clipboard

Add option to exclude keys from underscore/dash treatment?

Open wasnotrice opened this issue 6 years ago β€’ 8 comments

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?

wasnotrice avatar Sep 14 '18 21:09 wasnotrice

@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

doomspork avatar Oct 10 '18 20:10 doomspork

@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?

wasnotrice avatar Oct 10 '18 20:10 wasnotrice

@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 `

doomspork avatar Oct 10 '18 21:10 doomspork

@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"

wasnotrice avatar Oct 10 '18 21:10 wasnotrice

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?

doomspork avatar Oct 10 '18 21:10 doomspork

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.

wasnotrice avatar Oct 10 '18 21:10 wasnotrice

Sounds good to me @wasnotrice, a PR would be a great place to start πŸ‘

doomspork avatar Oct 10 '18 22:10 doomspork

@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!

snewcomer avatar Oct 11 '18 14:10 snewcomer

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.

protestContest avatar Feb 06 '24 00:02 protestContest

@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 avatar Feb 06 '24 15:02 mattpolzin

I'll call this issue closed by https://github.com/beam-community/jsonapi/pull/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.

mattpolzin avatar Feb 16 '24 01:02 mattpolzin