Support Rails 6.1 and 7.0 (DNM -- for reference only)

[lorint]

Seeing the performance advantages of the Trilogy driver compared with mysql2 has motivated me to make it compatible with older Rails versions. I'm hoping it can become the MySQL driver of choice in the Rails community. After trying several approaches, have ended up with the solution you see in this PR.

ActiveRecord Trilogy Adapter was originally written with a forward-thinking mind set -- one exclusive to Rails 7.1 and later. The older database adapter framework found in prior versions of Rails differs a fair bit from that found in 7.1, so there are some core differences to bridge in order to have Trilogy become compatible with prior Rails versions. This PR adds backwards compatibility in such a way that it does not interfere with the lean nature of how AR Trilogy has been architected -- it adds that compatibility layer via a series of patches all tucked away in a separate file. Inside that compatibility file, backwards_ar_compatibility.rb, checks are done to identify which newer methods are missing. They are selectively added back in, with the ultimate goal of establishing the broadest compatibility possible. Only those patches which are necessary for any given Rails version get applied, making it minimally invasive.

This approach requires no changes to Trilogy itself, so it functions in exactly the same way as it always has done -- as if everything is just ActiveRecord 7.1+. As such newer Rails 7.1 apps are completely unaffected because for them none of that patching needs be applied.

Eager to hear what you guys think!

[lorint]

Thank you @matthewd for triggering that first test run -- have fixed those two failures by simplifying how the connection gets made.

[lorint]

As to be expected, no test failures when running under ActiveRecord 7.1 as this update is inactive in that environment. When running under previous versions then the compatibility layer does get utilised. Here are the counts of errors and failures for the most recent release of each major AR version:

AR Version	failures	errors	skips
edge	0	0	0
7.0.4	1	3	0
6.1.7	2	5	2
6.0.6	2	8	2
5.2.8.1	11	8	2
5.1.7	11	13	2

[bensheldon]

@lorint Thanks for opening this PR! Our team will be able to do a better review of this after the New Years. I just wanted to give some quick feedback that I think would help expedite this work:

• I think 7.0 compatibility would take precedence, and it might be nice to split that out into a focused PR that we can review/approve before tackling older versions.
• I think we also need to have a multi-version local/CI test strategy and tooling. I have experience with Appraisal (on GoodJob) to make it easy to generate multiple gemfiles. I can work with you on that unless you (or someone else on my team) has stronger opinions.

btw, I also saw your post on Rails Discuss; thank you again!

[lorint]

• 7.0 compatibility would take precedence
• Have a multi-version local/CI test strategy and tooling

Sounds well, @bensheldon -- have now implemented Appraisal in the same way that you had done for GoodJob. Haven't touched the ci.yml file yet, and would be interested to have you or someone else familiar with GHA approach that one.

Have also pulled out support for 5.1 / 5.2 / 6.0. Being able to fix a couple more failures for 6.1 for the moment I've chosen to leave that in place, and let me know if you feel that should be a part of a different PR. Here's the current counts of errors / fails / skips:

AR Version	failures	errors	skips
unreleased	0	0	0
7.0.4	1	3	0
6.1.7	2	2	2

Getting down to the point that what's left are the more gnarly failures -- and overall they seem pretty inconsequential since Rails < 7.1 never had the connection verify logic in place, so even as it stands in some ways it's already stronger than just plain Rails would otherwise be. It only adds the 7.1 things for Trilogy and not for any other connection adapters.

Hope you folks are having a great end-of-year break, and eager to be back in touch when the team reconvenes!

[bensheldon]

@lorint thank you! And sorry, I realized now I sent you in one misdirection: there's already a ENV-based mechanism for switching the ActiveRecord version; I overlooked that so we don't need Appraisal here. I'll push up a commit to do that and update the CI to add the additional versions to the matrix.

Thank you for focusing on the more recent versions of Rails too. I'm still trying to draw up my engineers to take a look at this.

[lorint]

there's already a ENV-based mechanism for switching the ActiveRecord version

Cool! That does make it easy.

I've got some time tomorrow to try fixing the remaining test failures under 7.0 / 6.1 -- perhaps we won't need to put in any test skips. I don't think any of the current failures should be showstoppers anyway, just in case we do need to put in skips.

[lorint]

With this latest commit all examples should pass for AR 7.1 / 7.0 / 6.1.

Also have a set of patches ready for AR 6.0 that pass all examples, but there's more missing bits that need to be brought in for that kind of compatibility, so it adds another 80 lines. It is a currently supported Rails version, so in that light perhaps it's tempting to include. I'm totally onboard with providing older compatibility for the v2.2.0 release of this ARTA gem, and then removing all patches older than AR 6.1 for v2.3.0. This would make it easy for folks who want that older 6.0 compatibility to just bundle in v2.2.0 of ARTA.

Speaking of further backwards compatibility, after adding in the AR 6.0 patches then from there it is possible to further add support for 5.1 and 5.2, which requires about another 100 lines. Ends up being very functional, but does not pass a few examples. (Pretty tough to fully back-port all the synchronize / auto-reconnect stuff from AR 7.1 into 5.x.)

With all the patches enabled -- so that full additional 180 lines of stuff -- here's the current failure matrix:

AR Version	failures	errors	skips
edge	0	0	0
7.0.4	0	0	0
6.1.7	0	0	2
6.0.6	0	0	2
5.2.8.1	3	1	2
5.1.7	5	2	2

Alternatively could release all these 5.x / 6.0 patches as a separate "Trilogy compatibility" gem for anyone running those unsupported / nearly-unsupported versions of Rails. (sure, 6.0 is still supported, but for only another 4 months!) Would just ask that in the .gemspec for ARTA 2.2.0 it allows AR 5.0 and up. Otherwise such a compatibility gem would have to be a full fork of ARTA and not an add-on kind of thing.

Just to be totally clear here -- the failure matrix for the current code in this branch is for the first 3 lines of the table above -- zero failures for 6.1 / 7.0 / edge. The last 3 lines listed there are only opened up after adding another ~180 lines of various patches.

[bensheldon]

@lorint I did a review of this PR with @matthewd yesterday and we have some feedback:

• Thank you for looking into this and really reducing the problem-space down to these compatibility issues. Super helpful! 🙇🏻
• For the following, we want to put the work into a separate PR and keep this PR solely for reference.
• We don't want to do Active Record feature-patching. If other gems are using feature-detection (or their own feature patching) we're worried that it would be bad/unmaintainable.
• The strategy we'd like to go with is: Changing this adapter's interface/implementation using version checks (not feature detection) and not patching Active Record.
• We'd like to do that solely with a focus on Rails 7.0 in a separate PR and work through any feedback issues before considering any versions of Rails prior to that.

[lorint]

We'd like to (change this adapter's interface/implementation using version checks), solely with a focus on Rails 7.0 in a separate PR ...

Cool -- thanks @bensheldon and @matthewd for taking a look. I think it's great to have compatibility be more intrinsic to ARTA, so certainly this works. I recognise that supporting at least 7.0 will be vital for community adoption.

You might have sensed after looking through this PR that it is useful to implement the new AR 7.1 @verified flag, which is found as a part of ActiveRecord::ConnectionAdapters::AbstractAdapter. Of everything I had done this change was the most impactful -- it allows reconnection attempts work in the same way as AR 7.1. Ended up being the very most useful thing to start with in the journey. I expect if you can implement @verified early then everything else is more straightforward; really from there it becomes a bunch of minor details to sort out.

If you'd like, I have some time this weekend where I could start a new PR and take a stab at implementing @verified to see what you think.