rom-rb / rom-http

Abstract HTTP adapter for ROM
https://rom-rb.org
MIT License
73 stars 18 forks source link

Add support for `meta` in dataset #47

Open ianks opened 3 years ago

ianks commented 3 years ago

Sometimes, APIs like to key their response JSON in a weird way. For example, one API we use returns results in two ways:

{
  "results": []
}
{
  "customers": []
}

In order to account for this, it would be nice to have a way to attach meta on the dataset, as you can with the relation. That way, we can know the correct key to unwrap the response in the response handler.

Right now, we are hacking around this by storing the key in the params, which is not ideal.

Examples

dataset do
  with_meta(result_key: :customers)
end
ianks commented 3 years ago

Here's the proposed code diff, if y'all are cool with the idea, I'll make a PR for it:

module ROM
  module HTTP
    class Dataset
      option :meta, type: Types::Hash, default: proc { EMPTY_HASH }

      def add_meta(new_meta)
        with_options(meta: meta.merge(new_meta))
      end
    end

    class Relation
      forward :add_meta
    end
  end
end
solnic commented 3 years ago

@ianks this makes sense, as a side-note - IIRC it's something rom-elasticsearch would benefit from too, so I suspect this will end up in the core at some point.

ianks commented 3 years ago

@solnic should I PR to core?

solnic commented 3 years ago

@ianks let's start here and see how it goes 🙂

flash-gordon commented 3 years ago

I wonder if we can have an API for subclassing datasets and adding options there. That would probably work better because it prevents you from having an invalid dataset (invalid in your context I mean).

solnic commented 3 years ago

@flash-gordon you mean, from a rom user point of view, right?

flash-gordon commented 3 years ago

@solnic sure. Say, for a particular HTTP endpoint some parameters are required. It'd be good to have a type check of a kind to ensure you don't miss them when you're adding a new relation.

solnic commented 3 years ago

@flash-gordon hmm yeah this makes sense. This would have to be a setting at the Gateway level since it is the gateway that instantiates datasets for relations.