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

Columns filtering: blacklisting or whitelisting #16

Merged
merged 3 commits into from
Jan 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions lib/generators/logidze/install/templates/migration.rb.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,15 @@ class <%= @migration_class_name %> < ActiveRecord::Migration
SQL
end

<% if update? %>
execute <<-SQL
CREATE OR REPLACE FUNCTION logidze_version(v bigint, data jsonb) RETURNS jsonb AS $body$
DROP FUNCTION IF EXISTS logidze_version(bigint, jsonb);
DROP FUNCTION IF EXISTS logidze_snapshot(jsonb);
SQL
<% end %>

execute <<-SQL
CREATE OR REPLACE FUNCTION logidze_version(v bigint, data jsonb, blacklist text[] DEFAULT '{}') RETURNS jsonb AS $body$
Copy link
Owner

Choose a reason for hiding this comment

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

When upgrading we have to drop previously defined function, 'cause we change the signature.
So, we should add here DROP FUNCTION IF EXISTS ... in case of upgrade (use update? helper).

DECLARE
buf jsonb;
BEGIN
Expand All @@ -26,7 +33,7 @@ class <%= @migration_class_name %> < ActiveRecord::Migration
'v',
v,
'c',
logidze_exclude_keys(data, 'log_data')
logidze_exclude_keys(data, VARIADIC array_append(blacklist, 'log_data'))
);
IF coalesce(#{current_setting('logidze.responsible')}, '') <> '' THEN
buf := jsonb_set(buf, ARRAY['r'], to_jsonb(current_setting('logidze.responsible')));
Expand All @@ -36,12 +43,12 @@ class <%= @migration_class_name %> < ActiveRecord::Migration
$body$
LANGUAGE plpgsql;

CREATE OR REPLACE FUNCTION logidze_snapshot(item jsonb) RETURNS jsonb AS $body$
CREATE OR REPLACE FUNCTION logidze_snapshot(item jsonb, blacklist text[] DEFAULT '{}') RETURNS jsonb AS $body$
Copy link
Owner

Choose a reason for hiding this comment

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

The same goes here.

Copy link
Collaborator Author

@charlie-wasp charlie-wasp Jan 13, 2017

Choose a reason for hiding this comment

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

My bad, didn't think about it. Added DROP FUNCTION... statement 👍 Don't know why only one comment now is marked as outdated, though.

BEGIN
return json_build_object(
'v', 1,
'h', jsonb_build_array(
logidze_version(1, item)
logidze_version(1, item, blacklist)
)
);
END;
Expand Down Expand Up @@ -103,20 +110,22 @@ class <%= @migration_class_name %> < ActiveRecord::Migration
merged jsonb;
iterator integer;
item record;
columns_blacklist text[];
BEGIN
columns_blacklist := TG_ARGV[1];

IF TG_OP = 'INSERT' THEN

NEW.log_data := logidze_snapshot(to_jsonb(NEW.*));
NEW.log_data := logidze_snapshot(to_jsonb(NEW.*), columns_blacklist);

ELSIF TG_OP = 'UPDATE' THEN

IF OLD.log_data is NULL OR OLD.log_data = '{}'::jsonb THEN
NEW.log_data := logidze_snapshot(to_jsonb(NEW.*));
NEW.log_data := logidze_snapshot(to_jsonb(NEW.*), columns_blacklist);
RETURN NEW;
END IF;

history_limit := TG_ARGV[0];
history_limit := NULLIF(TG_ARGV[0], 'null');
current_version := (NEW.log_data->>'v')::int;

IF NEW = OLD THEN
Expand Down Expand Up @@ -149,7 +158,7 @@ class <%= @migration_class_name %> < ActiveRecord::Migration
NEW.log_data := jsonb_set(
NEW.log_data,
ARRAY['h', size::text],
logidze_version(new_v, changes),
logidze_version(new_v, changes, columns_blacklist),
true
);

Expand All @@ -174,9 +183,9 @@ class <%= @migration_class_name %> < ActiveRecord::Migration
def down
<% unless update? %>
execute <<-SQL
DROP FUNCTION logidze_version(bigint, jsonb) CASCADE;
DROP FUNCTION logidze_version(bigint, jsonb, text[]) CASCADE;
DROP FUNCTION logidze_compact_history(jsonb) CASCADE;
DROP FUNCTION logidze_snapshot(jsonb) CASCADE;
DROP FUNCTION logidze_snapshot(jsonb, text[]) CASCADE;
DROP FUNCTION logidze_logger() CASCADE;
SQL
<% end %>
Expand Down
39 changes: 39 additions & 0 deletions lib/generators/logidze/model/model_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,14 @@ class ModelGenerator < ::ActiveRecord::Generators::Base # :nodoc:
class_option :only_trigger, type: :boolean, optional: true,
desc: "Create trigger-only migration"

class_option :blacklist, type: :array, optional: true
class_option :whitelist, type: :array, optional: true

def generate_migration
if options[:blacklist] && options[:whitelist]
$stderr.puts "Use only one: --whitelist or --blacklist"
exit(1)
end
migration_template "migration.rb.erb", "db/migrate/#{migration_file_name}"
end

Expand Down Expand Up @@ -45,6 +52,38 @@ def backfill?
def only_trigger?
options[:only_trigger]
end

def columns_blacklist
array = if !options[:whitelist]
options[:blacklist]
else
class_name.constantize.column_names - options[:whitelist]
end

array || []
end

def logidze_logger_parameters
if limit.nil? && columns_blacklist.empty?
''
elsif !limit.nil? && columns_blacklist.empty?
limit
elsif !limit.nil? && !columns_blacklist.empty?
"#{limit}, #{format_pgsql_array(columns_blacklist)}"
elsif limit.nil? && !columns_blacklist.empty?
"null, #{format_pgsql_array(columns_blacklist)}"
end
end

def logidze_snapshot_parameters
return 'to_jsonb(t)' if columns_blacklist.empty?

"to_jsonb(t), #{format_pgsql_array(columns_blacklist)}"
end

def format_pgsql_array(ruby_array)
"'{" + ruby_array.join(', ') + "}'"
end
end

private
Expand Down
4 changes: 2 additions & 2 deletions lib/generators/logidze/model/templates/migration.rb.erb
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ class <%= @migration_class_name %> < ActiveRecord::Migration
CREATE TRIGGER logidze_on_<%= table_name %>
BEFORE UPDATE OR INSERT ON <%= table_name %> FOR EACH ROW
WHEN (coalesce(#{current_setting('logidze.disabled')}, '') <> 'on')
EXECUTE PROCEDURE logidze_logger(<%= limit || '' %>);
EXECUTE PROCEDURE logidze_logger(<%= logidze_logger_parameters %>);
SQL

<% if backfill? %>
execute <<-SQL
UPDATE <%= table_name %> as t
SET log_data = logidze_snapshot(to_jsonb(t));
SET log_data = logidze_snapshot(<%= logidze_snapshot_parameters %>);
SQL
<% end %>
end
Expand Down
3 changes: 2 additions & 1 deletion spec/dummy/db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20160415194001) do
ActiveRecord::Schema.define(version: 20170110144045) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
enable_extension "hstore"

create_table "posts", force: :cascade do |t|
t.string "title"
Expand Down
11 changes: 11 additions & 0 deletions spec/generators/model_generator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,17 @@ class User < ActiveRecord::Base
end
end

context "with columns blacklist" do
let(:args) { ["user", "--blacklist", "age", "active"] }

it "creates trigger with columns blacklist" do
is_expected.to exist
is_expected.to contain(
/execute procedure logidze_logger\(null, '\{age, active\}'\);/i
)
end
end

context "with backfill" do
let(:args) { ["user", "--backfill"] }

Expand Down
Loading