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 support for meta in dataset #47

Open
ianks opened this issue Feb 26, 2021 · 8 comments
Open

Add support for meta in dataset #47

ianks opened this issue Feb 26, 2021 · 8 comments
Labels

Comments

@ianks
Copy link
Contributor

ianks commented Feb 26, 2021

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 ianks added the feature label Feb 26, 2021
@ianks
Copy link
Contributor Author

ianks commented Feb 26, 2021

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
Copy link
Member

solnic commented Mar 1, 2021

@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
Copy link
Contributor Author

ianks commented Mar 4, 2021

@solnic should I PR to core?

@solnic
Copy link
Member

solnic commented Mar 5, 2021

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

@flash-gordon
Copy link
Member

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
Copy link
Member

solnic commented Mar 10, 2021

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

@flash-gordon
Copy link
Member

@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
Copy link
Member

solnic commented Mar 11, 2021

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

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

No branches or pull requests

3 participants