Skip to content

Lazy constantize relation resources#492

Merged
jkeen merged 5 commits into
graphiti-api:masterfrom
bguban:lazy_constantize_relation_resources
Mar 16, 2025
Merged

Lazy constantize relation resources#492
jkeen merged 5 commits into
graphiti-api:masterfrom
bguban:lazy_constantize_relation_resources

Conversation

@bguban

@bguban bguban commented Mar 1, 2025

Copy link
Copy Markdown
Contributor

When two resources have custom relation names it's hard to specify the resource for those relations because you need both resources to be loaded in memory. Doing it lazily solves the issue. You can specify a string as a resource name and it will be constantized later when needed

Comment thread lib/graphiti/sideload.rb Outdated

def resource_class
@resource_class ||= infer_resource_class
@cons_resource_class ||= (@resource_class.is_a?(String) ? @resource_class.constantize : @resource_class) ||

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkeen I'm not sure about the @cons_resource_class variable name but @resource_class directly assigns in line 52. Probably the best variant would be to rename @resource_class to something like @resource_class_name and then use @resource_class here for memorization. What do you think?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that sounds like a better approach, I agree.

What happens when the resource string you provide is invalid, though? Having the reference available by specifying the actual class name makes sure the reference is good before runtime, doesn't it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is how ActiveRecord does it for its relations. You set the relation class_name property as a string. It's done to omit a situation when one of the classes is not defined yet. Please pay attention to the test. When Ruby interprets UserResource the BookResource is not defined yet that's why if you try to use the class directly it causes an uninitialized constant error.
Usually, developers do not face this problem because infer_resource_class actually does the "lazy constantize" based on the relation name. But it can be a problem when you use custom relation names (like was done in the test) which is pretty common when building relations between models in different namespaces.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bguban I updated those instance variable names and the tests now pass

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkeen well done! Thank you! LGTM

jkeen and others added 3 commits March 15, 2025 11:38
@jkeen jkeen merged commit 3cc2983 into graphiti-api:master Mar 16, 2025
github-actions Bot pushed a commit that referenced this pull request Mar 16, 2025
## [1.7.8](v1.7.7...v1.7.8) (2025-03-16)

### Bug Fixes

* compare URI-decoded path params ([#482](#482)) ([20b80dd](20b80dd))
* correct issue with many_to_many when one of the models has a prefix to the intersection model association ([#449](#449)) ([dc28a4f](dc28a4f))
* lazy constantize relation resources ([#492](#492)) ([3cc2983](3cc2983))
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 1.7.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants