Add Alba::Railtie to set default inflector in Rails apps

[trevorturk]

Following the conversation from from #259 where we discuss setting Alba.inflector by default if you're using Rails (or perhaps also Hanami) I started looking into how best to implement the feature.

I ran into a problem because I wasn't sure where to actually set the default, since now we have Alba.inflector= but this can be called in a number of places. We could consider changing the code to use a method instead of attr_reader :inflector for example:

def inflector
  @inflector || set_default_inflector
end

def set_default_inflector
  if defined?(Rails)
    # ...
  end
end

However, there's a possible issue where you wouldn't be able to disable the inflector by setting it to nil. Perhaps this would be acceptable, or we could allow passing a :nil key or introducing Alba.inflector_disabled! or something like that. This is an open question, and I'd be happy to revisit.

I realized we also could consider using the Rails feature I believe is intended for this kind of use case: https://api.rubyonrails.org/classes/Rails/Railtie.html

This PR is a draft of what this would look like. I think it looks pretty good, but of course doesn't support Hanami. This could be a different feature, if we'd like to research or ping the Hanami team for advice. I'd be happy to provide support for this as well.

Finally, if we want to go farther with the Railtie idea, I can investigate how to embed a Rails app in the tests. I believe this is possible, but we'd need to require Rails for the test suite without adding it to the gemspec and making it a real dependency. This is something I believe I've seen done elsewhere, and I'd be happy to work on it. (I tested in my app locally, and this works as expected, but I'd be nervous to leave it untested in the gem.)

So, to summarize our options:

• If we don't like the Railtie idea, consider changing inflector from attr_reader to have a fallback inline
  • If so, is it acceptable if the user cannot "unset" the inflector (I think it's probably fine)
  • ...or would we want to introduce a new API like allowing passing :nil or disabling with a class method
• If we like the Railtie idea, consider adding tests to this PR's Railtie draft
  • If so, make sure not to require Rails when installing the gem (via the gemspec)
  • ...and embed a simple Rails app we can use to test all variants
• Once settled, update the README and CHANGELOG with examples

[codecov]

Codecov Report

Merging #274 (157894a) into main (81d27ae) will decrease coverage by 50.22%.
The diff coverage is 100.00%.

❗ Current head 157894a differs from pull request most recent head e403581. Consider uploading reports for the commit e403581 to get more accurate results

@@             Coverage Diff             @@
##             main     #274       +/-   ##
===========================================
- Coverage   99.38%   49.16%   -50.23%     
===========================================
  Files          12       11        -1     
  Lines         490      480       -10     
===========================================
- Hits          487      236      -251     
- Misses          3      244      +241     

Impacted Files	Coverage Δ
lib/alba.rb	56.47% <100.00%> (-43.53%)	⬇️
lib/alba/typed_attribute.rb	26.47% <0.00%> (-70.59%)	⬇️
lib/alba/association.rb	30.23% <0.00%> (-69.77%)	⬇️
lib/alba/nested_attribute.rb	40.00% <0.00%> (-60.00%)	⬇️
lib/alba/conditional_attribute.rb	40.00% <0.00%> (-60.00%)	⬇️
lib/alba/layout.rb	40.62% <0.00%> (-59.38%)	⬇️
lib/alba/resource.rb	53.38% <0.00%> (-46.20%)	⬇️
lib/alba/deprecation.rb	66.66% <0.00%> (-33.34%)	⬇️
lib/alba/default_inflector.rb

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[okuramasafumi]

@trevorturk Thank you!
I'd like to summarize pros and cons for those two ideas.

Railties:

• Pros
  • We can "on Rails"
  • The code is simple
• Cons
  • We might not be able to have the same implementation for Hanami and other frameworks

Non Railties:

• Pros
  • We can use the same code for all frameworks
• Cons
  • It's not possible to disable inflector with nil

I'd prefer Railties idea since I'd like to stick to inflector = nil API.

Do you know if Hanami has similar API?

[trevorturk]

I agree with your summary, thank you. I spent some time looking through the Hanami code (my first time!) but I wasn't able to find exactly what I was looking for. I think they may not have a system for this by design, but I also might be missing something since I'm new to the Hanami code. They have quite a nice system for registering providers etc, but wasn't able to automatically register a block to be run on app init as we have in the Railtie system.

I posted a question in the Hanami chat (https://gitter.im/hanami/chat?at=6373e656a34b511211315d01) and the discussion forum (https://discourse.hanamirb.org/t/can-a-3rd-party-gem-inject-itself-into-app-initialization/723) so perhaps we can wait to see if there's any advice.

It's worth noting that Hanami 2.0 is on a RC (release candidate) now, and I believe a lot has changed since 1.0. So, we may have a bit of a tricky problem trying to be compatible with Hanami 1.0 and 2.0 at the same time. In that case, we could condition things to only work by default in v2 and add docs about v1/v2.

So, a bit of a long post to say, let's see if we can get any help with the Hanami portion of this idea. In either case, we may want to consider:

• Testing out the non-Railties approach for both Rails and Hanami (I could work up a new PR for this)
• Keeping the Railties approach for Rails, and adding a nice clear section to the README for Hanami v1/v2

Let me know what you think. Thank you!

[okuramasafumi]

OK, let's wait for some response in a forum.
I think we don't need to support Hanami 1, because I believe the core team will not support it after version 2 is released.

If Hanami 2 provides a way to add some initialization step for a gem (which I believe is true in some way), I'd like to split setup code for both Rails and Hanami.
If not, we'll try non-Railtie approach. Still I'd like to keep Alba.inflector = nil API because we can check if inflection is enabled with if Alba.inflector which is intuitive.

So the code might look like:

def inflector
  @inflector == :initial ? set_default_inflector : @inflector
end

# reset is called at startup
def reset
  @inflector = :initial
end 

[trevorturk]

Agreed, this all sounds good. It's fun to review Hanami, so I'm glad to keep trying for this. I'm excited for the 2.0 release.

I wanted to say, another option would be to allow passing Alba.inflector = :nil or Alba.remove_inflector! or something like that.

The question I had overnight was: is there a real use case for having no inflector?

For example, if Alba tries to set the inflector automatically if possible (leaves it unset if not possible) but there's no way to unset it manually, is that a problem?

(Perhaps a user wants to disable the automatic inference feature, so then they'd need to unset the inflector?)

I just want to check that there's a real use case, otherwise we could consider not worrying about it for now, and we have the option to introduce a new method at a later time.

[trevorturk]

The Hamani team offered a suggestion here: https://discourse.hanamirb.org/t/can-a-3rd-party-gem-inject-itself-into-app-initialization/723

I gave it a try, but it didn't quite work as I expected. I provided details and example code so it's clear what I'm trying, in case they want to debug etc. I'll watch out for additional replies, but I think we're getting close!

[trevorturk]

Ok! So, I posted a longer explanation here but for some reason it's (hopefully temporarily) marked as spam.

The tl;dr is that I think we actually have everything hooked up correctly now, it's just that we didn't understand Hanami's boot process:

$ hanami console
bookshelf[development]> require 'alba'
=> true
bookshelf[development]> Hanami.boot
=> Bookshelf::App
bookshelf[development]> Alba.inflector
=> #<Dry::Inflector>

I didn't realize you'd need to call Hanami.prepare and/or Hanami.boot, and here I was expecting behavior that only happens when you call Hanami.boot. I think I understand what's happening now, and why Hanami was designed this way, so we should be good to go. I'll wait to see if they reply to confirm in any case, though.

For now, then, I wonder if you think I should try to write a couple of tests to validate the functionality. I believe I can embed a small Rails app and a small Hanami app to test against, but I'll have to do a little research to figure out the best way. Alternatively, we could go ahead without tests and just add documentation, but let me know what you think. (I'd say tests are preferred, and I'm happy to try to figure it out, but I don't mind skipping tests if you don't think we need them.)

[trevorturk]

No rush, but ping @okuramasafumi in case you hadn't seen the updates!

[okuramasafumi]

@trevorturk Sorry for late response, and thank you for the investigation!
I think we need to add some tests before adding new features. I don't want to put full Rails app inside test directory, so we'll need to figure out the way. Maybe adding one single "railtie" file is enough since we don't have to have actual "models"?

[trevorturk]

@okuramasafumi I'm sorry to say, but I haven't been able to get this working after trying for a while. I pushed an example where it might be close, using the rack-test gem, but I couldn't figure out how to make the embedded Rails application invoke the Railtie initializer.

It may be that more complicated setups with the appraisal are possible. It also seems you get some bootstrapping if you run rails plugin new myplugin --full to generate a new plugin with an embedded "dummy" app. But this all seems to complicated for me.

Perhaps it's possible to set something up on CI where we have 2 "dummy" apps which boot with independent Gemfiles and test somehow that way.

I'm sorry to say that I'm out of actionable ideas that are within my grasp at this point. Perhaps we should abandon the effort for now, unless someone has more ideas for us to try?

[okuramasafumi]

@trevorturk Thank you for trying things out. We can implement this feature without tests if we can be sure it works. I can confirm it works with Rails, but I have not used Hanami (1 or 2).

[trevorturk]

Let me test again to be sure and I'll report back.

An idea I had might be testing with CI somehow. I think there's a way to test GitHub Actions locally now, so I could give that a shot if you think it might be helpful. It wouldn't be as good as having as part of the normal test suite, but might be better than nothing. I'll give that a try, too, and let you know.

[okuramasafumi]

If it doesn't take much time, I think it's fine to run Railties test in both local and CI. We need to add test code to the repository anyway.
This is additional thought, but if we add Rails and Hanami dummy application to test directory, we can also test with ActiveRecord or ROM object as serialization targets.

[trevorturk]

@okuramasafumi I gave it a try with the idea I was thinking of, where you have a dummy Rails app that has it's own tests. It can have its own Gemfile which uses path to install the local copy of Alba. Then, I think we can get it running on CI. It's kind of weird, but maybe not a terrible idea? It's pretty realistic and we'd know if something broke via CI pretty easily.

In the tests you can see I left only one. I'm having a (separate?) issue where even if I (try to) disable Rails parallel testing, running the tests seems non-deterministic, meaning, they fail sometimes. I'm not sure what's going on with that. Something leaking in Ruby threads, perhaps? In any case, if you comment out Alba's Railtie code, the test will fail, so that's good news, and perhaps enough to go ahead with, I'm not sure.

So, please give this a review and see what you think. I believe we could figure out the same kind of system with a dummy Hanami app as well. We could also expand the tests in the Rails app if you wanted to really exercise things that way. I'm just not sure if this is a good idea in general, so I left things very simple to begin.

Thanks!

[trevorturk]

I'm seeing that CI didn't quite work. I'm going to try turning off bundler-cache re: ruby/setup-ruby#170 which brought me to: https://github.com/testdouble/test_data/blob/main/.github/workflows/ruby.yml -- this might be a more general strategy. We could also dig around in Justin Searls' gems a bit more to look for a nicer pattern. It seems we're not the other people that have needed something like this! I'm curious to hear your thoughts before I spend more time on it, but I think we're on the right track.

[okuramasafumi]

@trevorturk I saw CI failing due to the lack of rails installation. I think we need to put rails in the main Gemfile.
I also guess bundler-cache is not related to the problem we have, but I'm not sure.

[okuramasafumi]

https://api.rubyonrails.org/classes/ActiveSupport/LazyLoadHooks.html
This document might help to just use Railties for testing.

[trevorturk]

Yes, if you have some ideas please do try!

I was thinking of looking at Justin's gems for some ideas next, but he seems to use a script, which is a bit different from what I've seen, but might be smart.

I'm sure we can figure out some way to test this. Let me know what you think we should try next and I can spend some more time as well.

[trevorturk]

@okuramasafumi I'm closing this, but please feel free to reference as you work on the automatic Hanami setup. I'm happy to help test that as well. I still have my test Rails/Hanami apps locally.

[trevorturk]

Oh, and reminder that the Hanami team offered advice here, if you want to review: https://discourse.hanamirb.org/t/can-a-3rd-party-gem-inject-itself-into-app-initialization/723