Use middleware; include schema definitions in dump; minor cleanups

[ronen]

@stenver Here's an (awful big, sorry) PR right back at you. It's a bunch of mostly small commits, but since they mostly all depend on each other I didn't want to make them individual PRs. Here's the details:

c98f4ee remove ‘pry’ dependency:

I do use pry myself, but don't want to force all developers to use it

51584b7 include schema_plus_foreign_keys development dependency:

I needed to do that for the tests to run.

6f3f4e4 move tables implementation into middleware:

This was my bad: I had suggested modifying the connection adapter so that tables would return the schema-prefixed table names, which you did. But I had forgotten that schema_plus_core already defined a middleware stack for tables, which is the cleaner way to do it. So I moved your code over to be middleware.

e2e4a8a Simplify query code.

Once I was in there touching the query code, I noticed it was following boilerplate from ActiveRecord's PostgreSQL adapter, that handles arel queries -- but that was overkill in this case since the SQL is hardwired. So I simplified it a bit.

dc71f75 move suppress_messages into spec_helper:

A minor cleanup I've been doing gradually in the other SchemaPlus gems but figured I might as well do here as long as I'm at it.

ef79d64 Spec cleanup: add (multi)schema support methods:

Anticipating other other cleanups and new examples, I added spec/support/schema.rb that defines some helpers. In particular it defines with_schemas(['first', 'second']) do ... which encapsulates creating schemas, setting the search path, executing the code, then cleaning everything up -- removing those namespaces and their tables.

(OK I didn't really anticipate, I did this after the fact but then shuffled around the commits :)

7e972bc Spec cleanup: Make separate schema definitions for each context rather than one global schema for all examples:

You followed a pattern that's common in other schema_plus gem spec suites: make one big honkin' schema definition with every case in it, then have individual examples to verify the various features. But those spec suites are mostly legacy from a long time ago, and I haven't been through to clean them all up.

I think cleaner is to make a separate schema definition for each context, in order to test features individually. That makes the relationship between the schema and the tests more obvious, and makes the entire suite less fragile -- with one big schema definition, if you need something new for a new test you risk breaking the old tests by messing with the schema.

a759df7 Add spec for connection.tables

You originally wanted to do this in order to fix the dump; but the solution was to change connection.tables. And that change is an appropriate component of supporting multiple schemas. But it seemed to me that the new behavior of connection.tables should have its own test suite.

651d4b5 don’t add prefix when using default schema path

This is something that we didn't discuss. But a general goal for the schema_plus family of gems is that if their features aren't used, including them should be a no-op. So I thought this gem should suppress adding a prefix if the current schema path is PostgreSQL's default.

Does that seem OK to you?

3ab67a8 Added schema creation and search path

It occurred to me that just including the schema prefix in the dump makes a dump that's not really usable, since you couldn't do rake db:setup as the schemas wouldn't be defined. So (using more middleware) I included the CREATE SCHEMA IF NOT EXISTS and schema_search_path = statements at the top of the dump.

Does that also seem OK to you?

fb9ee2c Update doc to describe change to connection.tables, and including the schema setup in the dump.

Brought the README in line with all the above.

[stenver]

Thanks @ronen - very awesome that you took the time to make this massive improvement to this gem. Also thanks for the very well documented changes on commits - they were super helpful in understanding your thought process.

don’t add prefix when using default schema path
I think its fine - after all, if you are not using multiple schemas, then none of the features of this gem are required. It will also keep the code cleaner for the users of single schema.

3ab67a8 Added schema creation and search path
This is really my bad. Thanks for fixing it. When I was testing it out in another project, I never did rake db:setup, since I cannot use schema plus dump for database setup in there yet - we have custom types in our schema. I will make sure to create a mini project to properly test it out on the next time.

I have a few comments about the readme, but I can make it as a separate PR so it wouldnt hold back this person: SchemaPlus/schema_plus_views#10 (comment)