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

Ancestry Patches #17360

Merged
merged 6 commits into from
May 30, 2018
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
2 changes: 2 additions & 0 deletions app/models/orchestration_stack.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
require 'ancestry'
require 'ancestry_patch'

class OrchestrationStack < ApplicationRecord
require_nested :Status

Expand Down
1 change: 1 addition & 0 deletions app/models/relationship.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'ancestry'
require 'ancestry_patch'

class Relationship < ApplicationRecord
has_ancestry
Expand Down
1 change: 1 addition & 0 deletions app/models/service.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'ancestry'
require 'ancestry_patch'

class Service < ApplicationRecord
DEFAULT_PROCESS_DELAY_BETWEEN_GROUPS = 120
Expand Down
1 change: 1 addition & 0 deletions app/models/tenant.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'ancestry'
require 'ancestry_patch'

class Tenant < ApplicationRecord
HARDCODED_LOGO = "custom_logo.png"
Expand Down
70 changes: 70 additions & 0 deletions lib/patches/ancestry_patch.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
module AncestryInstanceMethodsPatch
def update_descendants_with_new_ancestry
super
unless ancestry_callbacks_disabled?
clear_memoized_instance_variables
if ancestry_changed? && !new_record? && sane_ancestry?
unscoped_descendants.each(&:clear_memoized_instance_variables)
end
end
end
end

module Ancestry
module InstanceMethods
prepend AncestryInstanceMethodsPatch

ANCESTRY_DELIMITER = '/'.freeze

def parse_ancestry_column(obj)
obj.to_s.split(ANCESTRY_DELIMITER).map! { |id| cast_primary_key(id) }
end

def ancestor_ids
@_ancestor_ids ||= parse_ancestry_column(read_attribute(ancestry_base_class.ancestry_column))
end

def parent_id
return @_parent_id if defined?(@_parent_id)
@_parent_id = if @_ancestor_ids
@_ancestor_ids.empty? ? nil : @_ancestor_ids.last
else
col = read_attribute(ancestry_base_class.ancestry_column)
# Specifically not using `.blank?` here because it is
# slower than doing the below.
if col.nil? || col.empty? # rubocop:disable Rails/Blank
Copy link
Member

Choose a reason for hiding this comment

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

I think I like col&.empty? - will remove the rails/blank comments and is faster

Copy link
Member Author

Choose a reason for hiding this comment

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

nope

Copy link
Member

Choose a reason for hiding this comment

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

so tell me. How do you really feel about &.? :trollface:

nil
else
rindex = col.rindex(ANCESTRY_DELIMITER)
cast_primary_key(rindex ? col[rindex + 1, col.length] : col)
end
end
end

def depth
@_depth ||= if @_ancestor_ids
@_ancestor_ids.size
else
col = read_attribute(ancestry_base_class.ancestry_column)
col ? col.count(ANCESTRY_DELIMITER) + 1 : 0
end
end

STRING_BASED_KEYS = %i[string uuid text].freeze
def cast_primary_key(key)
Copy link
Member

Choose a reason for hiding this comment

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

curious if the new casting changes are good enough to make this not necessary anymore

Copy link
Member Author

Choose a reason for hiding this comment

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

curious if the new casting changes are good enough to make this not necessary anymore

Possible. There are definitely things that are now obsolete in this patch that are fixed upstream.

That said, these are meant to be the minimal viable changes that can be shipped with MIQ with the current version of ancestry that we are using. My guess is that for backporting, we can't/shouldn't upgrade the version of ancestry we are using.

Not saying we shouldn't upgrade ancestry in master, but I see it as not feasible/safe to take what has been done in the gem and just merge it in for a backport. This patch is intended for fine/gaprindashvili.

if STRING_BASED_KEYS.include?(primary_key_type)
key
else
key.to_i
end
end

def clear_memoized_instance_variables
@_ancestor_ids = nil
@_depth = nil

# can't assign to `nil` since `nil` could be a valid result
remove_instance_variable(:@_parent_id) if defined?(@_parent_id)
end
end
end