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

Custom locator raises NameError when class name not defined #178

Open
daveolib opened this issue Mar 29, 2024 · 2 comments
Open

Custom locator raises NameError when class name not defined #178

daveolib opened this issue Mar 29, 2024 · 2 comments

Comments

@daveolib
Copy link

Problem here.

If I have a custom locator, say:

GlobalID::Locator.use :generated do |gid, options| 
  # Return generated object based on GID
end
...
GlobalID.parse("gid://generated/DoesNotExist/123").find

Before it gets to my locator, the library calls constantize on "DoesNotExist" when trying to get the model_class, even if there is no :only set in the options.

I'm not sure what the best solution is here -- if you skip find_allowed? when the :skip option isn't set, that introduces some subtle behaviour. It could be interesting to open another hook, say :use_class, which returns the class of the GID, but does not instantiate it. The find_allowed? portion would not be changed.

Thoughts?

@mreinsch
Copy link

mreinsch commented Apr 8, 2024

Ran into a similar issue just now, as I'm trying to use global IDs to store references to remote objects in a different service.

What I've decided doing at the moment is to delegate the model name resolution to the locator, as I want them to reside in their own ruby namespace to avoid (potential) conflicts with my app's models. That was the least intrusive patch I could find, if just requires patching the model_name method on GlobalID like this:

class GlobalID
  def model_name
    locator = GlobalID::Locator.send(:locator_for, self)
    locator.try(:model_name, uri) || uri.model_name
  end
end

This then allows me to register a locator like this:

module OtherApp
  RemoteObjectResponse = Data.define(:id, ...)

  class GidLocator
    def locate(gid, _options = {})
      api_client.get(gid.model_name, gid.model_id)
    end

    def model_name(uri)
      "OtherApp::#{uri.model_name}Response"
    end
  end
end

GlobalID::Locator.use :other_app, OtherApp::GidLocator.new

lookup works then with:

GlobalID.parse("gid://other_app/RemoteObject/123").find
#=> OtherApp::RemoteObjectResponse object

@mreinsch
Copy link

mreinsch commented Apr 9, 2024

BTW, the issue discussed here affects the example code in https://github.com/rails/globalid/blob/main/README.md#custom-app-locator.

For a real resolution in the project I think it'd be probably better to try and move the model class resolution/checking code from GobalID into the locators.

I can draft up a PR if that approach sounds acceptable?

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

2 participants