Lazily load default controller

[rickychilcott]

Bug fix

Description

This is a PR changes the way futurism's default_controller is resolved. Previously, you could only set as a constant (i.e. MyController), but now you can set as "MyController" and it will be converted to the constant upon use. This is to address a zietwork autoload issue as outlined in #68

NOTE: This is a commit that is based on #69 and shouldn't be merged until that PR is merged.

Fixes #68

Checklist

• My code follows the style guidelines of this project
• Checks (StandardRB & Prettier-Standard) are passing

[rickychilcott]

@scottbarrow do you want to try this locally to see if it resolves your params AND controller loading issues?

[scottbarrow]

@rickychilcott sorry for the delay, it does seem to fix the issue, however I have come across another issue that im not sure if its related.
in the locals hash, im passing in an array of 20 AR records, which seems to crash the signed_params message_verifier generator. @julianrubisch is there a limit to the size of the locals or the generated SGID?

[rickychilcott]

@scottbarrow I do think this is unrelated. @julianrubisch Have you experienced any issues with an exception raised from the message verifier?

https://api.rubyonrails.org/v6.0.3.3/classes/ActiveSupport/MessageVerifier.html doesn't mention any limit

[julianrubisch]

Sorry for being unresponsive, too. I think the error message „singleton can’t be dumped“ points in another direction.

MessageVerifier uses Marshal which obviously doesn’t allow serializing of Singletons:

http://www.asherkory.com/Singletons-and-Caching/

https://stackoverflow.com/questions/31291363/singleton-cant-be-dumped-cached-resource-gem

[scottbarrow]

Hey guys, has there been any progress on this, currently we're not able to implement futurism due to this and the params issue reported, please let me know how I can help out

[rickychilcott]

I haven't had a chance to update this PR based on the linked stuff -- I'm just getting back to work after over 2 months. I'm hoping to get some time next week (I know I said that LAST week). Sorry. ᐧ

…

On Fri, Nov 6, 2020 at 5:23 PM Scott Barrow ***@***.***> wrote: Hey guys, has there been any progress on this, currently we're not able to implement futurism due to this and the params issue reported, please let me know how I can help out — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#70 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADM2P4XX2BP3ZKSU4XBPSLSORZOXANCNFSM4SQ76K7Q> .

-- Ricky Chilcott Partner Mission Met (740) 561-1361 ricky@missionmet.com www.missionmet.com

[julianrubisch]

Hey @scottbarrow sorry for the delay, it has been quite a month here, too.

The problem I see here is one of reproduction. I easily futurize > 20 AR on a regular basis - also because futurism actually splits that up into multiple placeholders anyway. GlobalID would theoretically support locate_many, but we haven’t incorporated that yet.

So I fear it’s your turn to provide us with an MVCE :-(

[rickychilcott]

Once #69 is merged, I can rework this PR to be just the appropriate pieces and try to get this ready to be reviewed.

[julianrubisch]

Please rebase!

[rickychilcott]

Done!

[julianrubisch]

Is it ready to be tested, then?

[rickychilcott]

Yep. It should be ready to be tested and for prime time. I tested it manually, and have some tests in place to ensure the setter is used appropriately.

[julianrubisch]

Great, will put it through the paces tomorrow... don’t see any blockers though

[julianrubisch]

Works as advertised, thanks!

Just a semver question: given that we change the semantics of how to specify the default_controller, this should "at least" be a minor version bump, correct?

[rickychilcott]

Good question on the version number. Since it won't break if they made it an actual class name versus a string -- I think it'd be ok to release as just a patch, but the recommendation is now to set it as a string. Since this is a non-breaking change, I don't see the need to do a minor bump, but it might communicate "hey something more significant has changed about this gem".

Up to you.

[julianrubisch]

Ah, because of .to_s.constantize... gotcha 👍🏻