Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for setting meta without transaction block #143

Merged
merged 10 commits into from
Nov 29, 2019
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,24 @@

# Change log

## master (unreleased)
- PR [#143](https://github.com/palkan/logidze/pull/143) Add `:transactional` option to `#with_meta` and `#with_responsible` ([@oleg-kiviljov][])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more thing: please, add the value for your nickname's shortcut to the end of this file.


Now it's possible to set meta and responsible without wrapping the block into a DB transaction. For backward compatibility `:transactional` option by default is set to `true`.

Usage:

```ruby
Logidze.with_meta({ip: request.ip}, transactional: false) do
post.save!
end
```
or
```ruby
Logidze.with_responsible(user.id, transactional: false) do
post.save!
end
```

## 0.11.0 (2019-08-15)

Expand Down Expand Up @@ -284,3 +302,4 @@ This is a quick fix for a more general problem (see [#59](https://github.com/pal
[@dmitrytsepelev]: https://github.com/DmitryTsepelev
[@zocoi]: https://github.com/zocoi
[@duderman]: https://github.com/duderman
[@oleg-kiviljov]: https://github.com/oleg-kiviljov
15 changes: 13 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,13 @@ end

Meta expects a hash to be passed so you won't need to encode and decode JSON manually.

**NOTE**: `.with_meta` wraps the block into a DB transaction (see https://github.com/palkan/logidze/issues/136).
By default `.with_meta` wraps the block into a DB transaction. That could lead to an unexpected behavior, especially, when using `.with_meta` within an around_action. To avoid wrapping the block into a DB transaction use `transactional: false` option.

```ruby
Logidze.with_meta({ip: request.ip}, transactional: false) do
post.save!
end
```

## Track responsibility (aka _whodunnit_)

Expand Down Expand Up @@ -306,8 +312,13 @@ class ApplicationController < ActionController::Base
end
```

**NOTE**: `.with_responsible` wraps the block into a DB transaction (see https://github.com/palkan/logidze/issues/136).
By default `.with_responsible` wraps the block into a DB transaction. That could lead to an unexpected behavior, especially, when using `.with_responsible` within an around_action. To avoid wrapping the block into a DB transaction use `transactional: false` option.

```ruby
Logidze.with_responsible(user.id, transactional: false) do
post.save!
end
```
## Disable logging temporary

If you want to make update without logging (e.g., mass update), you can turn it off the following way:
Expand Down
59 changes: 43 additions & 16 deletions lib/logidze/meta.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,19 @@
module Logidze # :nodoc:
# Provide methods to attach meta information
module Meta
def with_meta(meta, &block)
MetaTransaction.wrap_with(meta, &block)
def with_meta(meta, transactional: true, &block)
palkan marked this conversation as resolved.
Show resolved Hide resolved
wrapper = transactional ? MetaWithTransaction : MetaWithoutTransaction
wrapper.wrap_with(meta, &block)
end

def with_responsible(responsible_id, &block)
def with_responsible(responsible_id, transactional: true, &block)
return yield if responsible_id.nil?

meta = {Logidze::History::Version::META_RESPONSIBLE => responsible_id}
with_meta(meta, &block)
with_meta(meta, transactional: transactional, &block)
end

class MetaTransaction # :nodoc:
class MetaWrapper # :nodoc:
def self.wrap_with(meta, &block)
new(meta, &block).perform
end
Expand All @@ -29,25 +30,22 @@ def initialize(meta, &block)
end

def perform
return if block.nil?
raise ArgumentError, "Block must be given" unless block
return block.call if meta.nil?

ActiveRecord::Base.transaction { call_block_in_meta_context }
call_block_in_meta_context
end

private

def call_block_in_meta_context
prev_meta = current_meta

meta_stack.push(meta)

pg_set_meta_param(current_meta)
result = block.call
pg_reset_meta_param(prev_meta)

result
ensure
pg_reset_meta_param(prev_meta)
meta_stack.pop
end

Expand All @@ -60,20 +58,49 @@ def meta_stack
Thread.current[:meta]
end

def pg_set_meta_param(value)
encoded_meta = connection.quote(ActiveSupport::JSON.encode(value))
connection.execute("SET LOCAL logidze.meta = #{encoded_meta};")
def encode_meta(value)
connection.quote(ActiveSupport::JSON.encode(value))
end

def pg_reset_meta_param(prev_meta)
if prev_meta.empty?
connection.execute("SET LOCAL logidze.meta TO DEFAULT;")
pg_clear_meta_param
else
pg_set_meta_param(prev_meta)
end
end
end

private_constant :MetaTransaction
class MetaWithTransaction < MetaWrapper # :nodoc:
private

def call_block_in_meta_context
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, we can move this to the base class (MetaWrapper).

connection.transaction { super }
end

def pg_set_meta_param(value)
connection.execute("SET LOCAL logidze.meta = #{encode_meta(value)};")
end

def pg_clear_meta_param
connection.execute("SET LOCAL logidze.meta TO DEFAULT;")
end
end

class MetaWithoutTransaction < MetaWrapper # :nodoc:
private

def pg_set_meta_param(value)
connection.execute("SET logidze.meta = #{encode_meta(value)};")
end

def pg_clear_meta_param
connection.execute("SET logidze.meta TO DEFAULT;")
end
end

private_constant :MetaWrapper
private_constant :MetaWithTransaction
private_constant :MetaWithoutTransaction
end
end
22 changes: 22 additions & 0 deletions spec/integrations/meta_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,28 @@
expect(subject.at_version(3).meta).to eq meta2
end
end

context "when transactional:false" do
it "resets meta setting after block finishes" do
# subject is a newly created user
Logidze.with_meta(meta, transactional: false) do
expect(subject.reload.meta).to eq meta
end

# create another one and check that meta is nil here
expect(User.create!(name: "test", age: 10, active: false).reload.meta).to be_nil
end

it "recovers after exception" do
ignore_exceptions do
Logidze.with_meta(meta, transactional: false) do
CustomUser.create!
end
end

expect(subject.reload.meta).to be_nil
end
end
end

describe ".with_responsible" do
Expand Down