Full support for AR5

[boazy]

Fixes #9

I've managed to get all tests passing with this PR, but in the price of removing AR4.2 support.
Due to the differences between AR4.2 and AR5, supporting both of them simultaneously is going to complicate the code and I'm not sure I'm up to that task.

It's probably best to just start a new branch for AR5, since it's it's different enough.

All the tests are green now, but there are probably some corner cases left which are not covered and could surface later with dependencies on schema_plus_core.
I've removed monkey patching of Dumper::Indexes (since indexes are now created inline in create_table), but it seems like materialized views are not using it, although It seems like schema_plus_views doesn't support them anyway, so I left that out for now.

[coveralls]

Coverage decreased (-0.3%) to 99.746% when pulling e79adc4 on boazy:activerecord-5.0 into a5310a6 on SchemaPlus:activerecord-5.0.

[coveralls]

Coverage decreased (-1.4%) to 98.615% when pulling 56517b8 on boazy:activerecord-5.0 into a5310a6 on SchemaPlus:activerecord-5.0.

[coveralls]

Coverage decreased (-1.4%) to 98.615% when pulling 56517b8 on boazy:activerecord-5.0 into a5310a6 on SchemaPlus:activerecord-5.0.

[ronen]

@boazy you're a hero! I agree it makes sense to make an AR5 branch -- or even better, make an AR4 branch for any updates to old code, and use master for AR5.

Any reason this couldn't be a minor version bump (1.1.0) rather than major (2.0.0)? Are there any backwards-incompatible changes? If there are no incompatible changes, I'd go with 1.1.0 and let bundler resolve the dependencies to choose 1.0.x for AR 4.2 and >= 1.1.0 for AR 5.x

My quick tests show that schema_plus_foreign_keys and schema_plus_indexes fail under AR 5.0 even with your updated schema_plus_core, though that's probably due to problems directly in those gems rather than here in schema_plus_core.

Since the tests pass, I'm tempted to simply release your version (with a note in the README indicating that AR 4.2 support ended with 1.0.x), then go on to look into the other gems in the family. If it turns out there are corner cases not covered by the tests, we could then go back and cut a patch release. More conservative would be to cut a prerelease (1.1.0.pre or 2.0.0.pre) but that just seems like it'd add complications for no great benefit. Your thoughts?

BTW this contribution completely qualifies you to be a member of the SchemaPlus github team so you can work without requiring me to be in the loop. If you're interested let me know.

Thanks!

[boazy]

Thanks @ronen!

It seems most gems depending on schema_plus_core specify "~> 1.0", and this version definitely breaks backward compatibility by pushing a dependency on AR5 when auto-updating. I think the GemSpec still accepts AR4.2, but the gem would fail with that version now. I could try to detect AR4.2 and change the behavior accordingly, but that would make the change more complicated than I'm comfortable with.

In the meantime, I've managed to get schema_plus_foreign_keys to pass the tests (at least on my machine) with very little modifications - essentially all the failures stem from expecting deprecation warnings, but not preparing for the new deprecation warning raised by using ConnectionAdapter#tables and ConnectionAdapter#table_exists?. I've replaced them with data_sources wherever possible and added expectation to the new deprecation warnings when the call is internal to AR5.

I'm now trying to fix schema_associations, and it gets a lot of failures I don't yet understand. I'm pretty sure most of the gems dependent on schema_plus_core are going to fail initially with AR5, if only due to the new deprecation warnings, so they will all need to be bumped. :(

I'd be happy to have a proper release anyway, since it will make fixing the rest of the gems down the chain easier. :)

Thanks for adding me to the team!

[ronen]

It seems most gems depending on schema_plus_core specify "~> 1.0", and this version definitely breaks backward compatibility by pushing a dependency on AR5 when auto-updating.

...and even more so (I finally had time to look through it) it changes the middleware API that the gem exports, renaming SchemaMonkey::Middleware::Schema::Tables to SchemaMonkey::Middleware::Schema::DataSources. I had to think a moment about that... the main purpose of schema_plus_core is to provide a stable API for client gems, that can stay constant despite internal changes to AR implementation. But I'd say that the change from AR4 to AR5 is an external change rather than just an internal implenetation change, so it warrants a corresponding change in schema_plus_core.

Could you update the README to reflect that change, as well as the additional parameter to Query::Exec, and any other API changes? Then I'll quick as I can merge this and cut the 2.0 release.

[boazy]

I'm pretty sure SchemaMonkey::Middleware::Schema::Tables would be back in the future, but it would be back with different semantics, which is why the deprecation warning is raised at the first place. I also hesitated changing that, but in the end you can't protect the users from the change AR is planning to introduce (apparently starting with Rails 5.1).

And, I'll definitely update the readme. I should have done that earlier, but I just wanted to see whether change is okay.

Thanks for the support!

[coveralls]

Coverage decreased (-0.3%) to 99.745% when pulling d99c343 on boazy:activerecord-5.0 into a5310a6 on SchemaPlus:activerecord-5.0.