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

Ability to make global id without loading the object #181

Open
nhorton opened this issue Aug 6, 2024 · 5 comments
Open

Ability to make global id without loading the object #181

nhorton opened this issue Aug 6, 2024 · 5 comments

Comments

@nhorton
Copy link

nhorton commented Aug 6, 2024

I have some use cases where I need to do almost exactly what is in your docs - MyModel.find(x).to_global_id. That find(x) is a bunch of extra work that I would like to avoid. It would be really nice if I could do MyModel.global_id_for(x) and get the global ID that way. Granted, if x was a bad ID, the global_id would be bad, but that is obviously my bug.

@shettytejas
Copy link

shettytejas commented Aug 29, 2024

Hey @nhorton, I’m trying to understand the exact use case for needing this method. I am assuming it's one of these two scenarios where you might be getting the x (ID):

  1. You already have the object: If you already have the object whose GlobalID you need, it's better to pass that object around rather than constructing a GlobalID from the ID yourself.

  2. The ID comes from user input or similar sources: This is a security risk because GlobalID fetches objects directly, bypassing scopes and any security checks. If a bad ID is passed, it could lead to data leaks or unauthorised access.

If there's any other use case that I may have missed, feel free to call it out!

@nhorton
Copy link
Author

nhorton commented Aug 29, 2024

Hi!
The scenario is that we are serializing some information about associations, and are doing extra lookups. My app domain is pretty complex, but I will give an analogous example below:

Imagine you have an online store that sells many products, but they are different enough that you have multiple tables: books, clothes, shoes, etc.

When a user makes a purchase, we want to record all the products that they looked at in the record along with what they bought and store that. The main product purchased was done with a polymorphic association, so it is fine. We don't want to store the IDs of the items viewed because we would get something like JSON where we kept adding new keys for each table array values of the IDs, and for some reasons of how this record is viewed in other places, that would be problematic. Instead, we want to have an array of global IDs. But as it stands, we have to fetch all the products viewed by ID just so that we can generate the GlobalID for each one. If we could generate the global ID for each from the ID alone (and the class obviously) we could do this without touching the DB for all those records.

@shettytejas
Copy link

shettytejas commented Aug 29, 2024

@nhorton, that makes sense to me, but I'm still unsure about a few things. However, those are more application-related concerns rather than library-specific ones. 😄

I've opened a PR (#186) that adds support for building GlobalId.

If you need this solution urgently, feel free to use the following monkey patch:

# lib/patches/global_id/identification.rb - Make sure you require this file in initializer.
module GlobalId::Identification
  class << self
    def included(base)
      base.extend(ClassMethods)
    end
  end

  module ClassMethods
    def build_global_id(obj, options = {})
      return obj.to_global_id(options) if obj.respond_to?(:to_global_id)
      raise ArgumentError, "Can't build a Global ID for #{obj.class}" unless obj.is_a?(String) || obj.is_a?(Integer)

      unless (app = options.fetch(:app) { GlobalID.app })
        raise ArgumentError, "An app is required to create a GlobalID. Pass the :app option or set the default GlobalID.app."
      end

      GlobalID.new(URI::GID.build(app: app, model_name: name, model_id: obj, params: options.except(:app, :verifier, :for)))
    end
  end
end

@nhorton
Copy link
Author

nhorton commented Aug 31, 2024

This is great! Thank you so much.

@nflorentin
Copy link

nflorentin commented Oct 9, 2024

That would be a great feature.

I have the following simple use case :

  • a recurring job which find orders not paid and enqueue a sync job for each order
  • a job to sync order

The two jobs are separated because the job to sync order make API calls so if API call fails, we want to retry only the job associated to the specific order.

I could do that:

class RecurringJob 
  def perform
    Order.not_paid.each { |order| SyncJob.perform_later(order) }
  end
end

But I'll be instantiating a lot of Order objects for nothing.

I prefer to do that:

class RecurringJob 
  def perform
    Order.not_paid.pluck(:id).each { |order_id| SyncJob.perform_later(order_id) }
  end
end

But then, I can't benefit from automatic global id deserialization in SyncJob.

I would like to do that:

class RecurringJob 
  def perform
    Order.not_paid.pluck(:id).each { |order_id| SyncJob.perform_later(Order.build_global_id(order_id)) }
  end
end

Thanks for the patch, although I will wait for a version of globalid with your pull request because I'm not a huge fan of monkey-patching gems.

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

No branches or pull requests

3 participants