rom-http icon indicating copy to clipboard operation
rom-http copied to clipboard

Add support for `meta` in dataset

Open ianks opened this issue 4 years ago • 8 comments

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 avatar Feb 26 '21 16:02 ianks

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

ianks avatar Feb 26 '21 16:02 ianks

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

solnic avatar Mar 01 '21 05:03 solnic

@solnic should I PR to core?

ianks avatar Mar 04 '21 16:03 ianks

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

solnic avatar Mar 05 '21 05:03 solnic

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).

flash-gordon avatar Mar 10 '21 10:03 flash-gordon

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

solnic avatar Mar 10 '21 10:03 solnic

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

flash-gordon avatar Mar 10 '21 11:03 flash-gordon

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

solnic avatar Mar 11 '21 05:03 solnic