Commit Graph

14 Commits

Author SHA1 Message Date
Ryuta Kamizono c81af6ae72 Enable `Layout/EmptyLinesAroundAccessModifier` cop
We sometimes say "✂️ newline after `private`" in a code review (e.g.
https://github.com/rails/rails/pull/18546#discussion_r23188776,
https://github.com/rails/rails/pull/34832#discussion_r244847195).

Now `Layout/EmptyLinesAroundAccessModifier` cop have new enforced style
`EnforcedStyle: only_before` (https://github.com/rubocop-hq/rubocop/pull/7059).

That cop and enforced style will reduce the our code review cost.
2019-06-13 12:00:45 +09:00
Ryuta Kamizono 56128abcba Test prepared statement cache only if prepared statements is enabled 2018-11-09 00:32:36 +09:00
Kir Shatrov 831be98f9a Use frozen-string-literal in ActiveRecord 2017-07-19 22:27:07 +03:00
Matthew Draper 87b3e226d6 Revert "Merge pull request #29540 from kirs/rubocop-frozen-string"
This reverts commit 3420a14590, reversing
changes made to afb66a5a59.
2017-07-02 02:15:17 +09:30
Kir Shatrov cfade1ec7e Enforce frozen string in Rubocop 2017-07-01 02:11:03 +03:00
Ryuta Kamizono 3464cd5c28 Fix broken comments indentation caused by rubocop auto-correct [ci skip]
All indentation was normalized by rubocop auto-correct at 80e66cc4d9.
But comments was still kept absolute position. This commit aligns
comments with method definitions for consistency.
2016-09-14 18:26:32 +09:00
Xavier Noria 80e66cc4d9 normalizes indentation and whitespace across the project 2016-08-06 20:16:27 +02:00
Xavier Noria 9617db2078 applies new string literal convention in activerecord/test
The current code base is not uniform. After some discussion,
we have chosen to go with double quotes by default.
2016-08-06 18:26:53 +02:00
Sam Davies 9e28f415f5 Make tests a bit more beautiful 2015-11-05 17:44:47 -03:00
Sam Davies 50c5334082 Correctly deallocate prepared statements if we fail inside a transaction
- Addresses issue #12330

Overview
========

Cached postgres prepared statements become invalidated if the schema
changes in a way that it affects the returned result.

Examples:
- adding or removing a column then doing a 'SELECT *'
- removing the foo column  then doing a 'SELECT bar.foo'

In normal operation this isn't a problem, we can rescue the error,
deallocate the prepared statement and re-issue the command.

However in PostgreSQL transactions, once any command fails, the
transaction becomes 'poisoned' and any subsequent commands will raise
InFailedSQLTransaction.

This includes DEALLOCATE statements, so the default deallocation
strategy instead of removing the cached prepared statement instead
raises InFailedSQLTransaction.

Why this is bad
===============

1. InFailedSQLTransaction is a fairly cryptic error and doesn't
communicate any useful information about what has actually gone wrong.
2. In the naive implementation the prepared statement never gets
deallocated - it stays alive for the length of the session taking up
memory on the postgres server.
3. It is unsafe to retry the transaction because the bad prepared
statement is still in the cache and we would see the exact same failure
repeated.

Solution
========

If we are outside a transaction we can continue to handle these failures
gracefully in the usual way.

Inside a transaction instead of issuing a DEALLOCATE command that will
certainly fail, we now raise
ActiveRecord::PreparedStatementCacheExpired.

This can be handled further up the stack, notably inside
TransactionManager#within_new_transaction. Here we can make sure to
first rollback the transaction, then safely issue DEALLOCATE statements
to invalidate the rest of the cached prepared statements.

This also allows the user (or some gem) the opportunity to catch this error and
voluntarily retry the transaction if a schema change causes the prepared
statement cache to become invalidated.

Because the outdated statement has been deallocated, we can expect the
transaction to succeed on the second try.
2015-11-05 17:39:02 -03:00
Brandon Weiss 0965863564 Closes rails/rails#18864: Renaming transactional fixtures to transactional tests
I’m renaming all instances of `use_transcational_fixtures` to
`use_transactional_tests` and “transactional fixtures” to
“transactional tests”.

I’m deprecating `use_transactional_fixtures=`. So anyone who is
explicitly setting this will get a warning telling them to use
`use_transactional_tests=` instead.

I’m maintaining backwards compatibility—both forms will work.
`use_transactional_tests` will check to see if
`use_transactional_fixtures` is set and use that, otherwise it will use
itself. But because `use_transactional_tests` is a class attribute
(created with `class_attribute`) this requires a little bit of hoop
jumping. The writer method that `class_attribute` generates defines a
new reader method that return the value being set. Which means we can’t
set the default of `true` using `use_transactional_tests=` as was done
previously because that won’t take into account anyone using
`use_transactional_fixtures`. Instead I defined the reader method
manually and it checks `use_transactional_fixtures`. If it was set then
it should be used, otherwise it should return the default, which is
`true`. If someone uses `use_transactional_tests=` then it will
overwrite the backwards-compatible method with whatever they set.
2015-03-16 11:35:44 -07:00
Aaron Patterson 91d3d00240 fix tests for explain plan + binds 2014-01-13 16:42:12 -08:00
Aaron Patterson d220e8add9 recover from test runs that leave the database in a bad state 2014-01-13 15:02:28 -08:00
Jon Leighton d14da79334 Add an explicit test for hot compatibility 2012-09-28 18:08:14 +01:00