Skip to content

Remove contracts from entitlement calculation hot paths #78

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.

Sign up for GitHub

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

Open
hosom wants to merge 1 commit into
base: hosom/optimize-calculation-logic
Choose a base branch
from
Open

Remove contracts from entitlement calculation hot paths #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 4 additions & 18 deletions lib/entitlements/data/groups/calculated.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
# Calculate groups that should exist and the contents of each based on a set of rules
# defined within a directory. The calculation of members is global across the entire
# entitlements system, so this is a singleton class.
#
# Calculation methods intentionally use explicit validation instead of runtime contracts.
# These methods recurse over large collections, and collection contracts revalidate every
# member on each cached return.

module Entitlements
class Data
Expand Down Expand Up @@ -46,7 +50,6 @@ def self.reset!
# Takes no arguments.
#
# Returns a Entitlements::Models::Group object.
Contract String => Entitlements::Models::Group
def self.read(dn)
return @groups_cache[dn] if @groups_cache[dn]
raise "read(#{dn.inspect}) does not support calculation at this time. Please use read_all() first to build cache."
Expand All @@ -59,9 +62,6 @@ def self.read(dn)
# cfg_obj - Hash with the configuration for that key from the configuration file.
#
# Returns a Set of Strings (DNs) of the groups in this OU.
Contract String, C::HashOf[String => C::Any], C::KeywordArgs[
skip_broken_references: C::Optional[C::Bool]
] => C::SetOf[String]
def self.read_all(ou_key, cfg_obj, skip_broken_references: false)
return read_mirror(ou_key, cfg_obj) if cfg_obj["mirror"]

Expand Down Expand Up @@ -118,7 +118,6 @@ def self.read_all(ou_key, cfg_obj, skip_broken_references: false)
#
# Returns a hash { dn => Entitlements::Models::Group }
# :nocov:
Contract C::None => C::HashOf[String => Entitlements::Models::Group]
def self.to_h
@groups_cache
end
Expand All @@ -129,7 +128,6 @@ def self.to_h
# Takes no arguments.
#
# Returns a Hash of OU to the configuration and group objects it contains.
Contract C::None => C::HashOf[String => { config: C::HashOf[String => C::Any], groups: C::HashOf[String => Entitlements::Models::Group]}]
def self.all_groups
@groups_in_ou_cache.map do |ou_key, dn_in_ou|
if @config_cache.key?(ou_key)
Expand All @@ -154,7 +152,6 @@ def self.all_groups
# cfg_obj - Hash with the configuration for that key from the configuration file.
#
# Returns a Set of Strings (DNs) of the groups in this OU.
Contract String, C::HashOf[String => C::Any] => C::SetOf[String]
def self.read_mirror(ou_key, cfg_obj)
@groups_in_ou_cache[ou_key] ||= begin
Entitlements.logger.debug "Mirroring #{ou_key} from #{cfg_obj['mirror']}"
Expand Down Expand Up @@ -184,15 +181,6 @@ def self.read_mirror(ou_key, cfg_obj)
# filename - A String with the filename.
#
# Returns an Entitlements::Data::Groups::Calculated::* object.
Contract C::KeywordArgs[
filename: String,
config: C::HashOf[String => C::Any],
options: C::Optional[C::HashOf[Symbol => C::Any]]
] => C::Or[
Entitlements::Data::Groups::Calculated::Ruby,
Entitlements::Data::Groups::Calculated::Text,
Entitlements::Data::Groups::Calculated::YAML,
]
def self.ruleset(filename:, config:, options: {})
unless filename =~ /\.(\w+)\z/
raise ArgumentError, "Unable to determine the extension on #{filename.inspect}!"
Expand Down Expand Up @@ -242,7 +230,6 @@ def self.file_object_key(filename)
# Takes no arguments.
#
# Returns a Hash.
Contract C::None => C::HashOf[String => Class]
def self.rules_index
@rules_index
end
Expand All @@ -252,7 +239,6 @@ def self.rules_index
# Takes no arguments.
#
# Returns a Hash.
Contract C::None => C::HashOf[String => Object]
def self.filters_index
@filters_index
end
Expand Down
25 changes: 0 additions & 25 deletions lib/entitlements/data/groups/calculated/base.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ class Base
# Takes no arguments.
#
# Returns Set[Entitlements::Models::Person] of all matching members.
Contract C::None => C::SetOf[Entitlements::Models::Person]
def members
# :nocov:
raise "Must be implemented in child class"
Expand All @@ -51,7 +50,6 @@ def members
# Takes no arguments.
#
# Returns a String.
Contract C::None => String
def description
# :nocov:
raise "Must be implemented in child class"
Expand All @@ -64,7 +62,6 @@ def description
#
# Returns Hash[<String>key => <Object>value]
# :nocov:
Contract C::None => C::HashOf[String => C::Any]
def modifiers
{}
end
Expand All @@ -78,11 +75,6 @@ def modifiers
#
# filename - Filename with the ruleset.
# options - An optional hash of additional options.
Contract C::KeywordArgs[
filename: String,
config: C::Maybe[C::HashOf[String => C::Any]],
options: C::Optional[C::HashOf[Symbol => C::Any]]
] => C::Any
def initialize(filename:, config: nil, options: {})
@filename = filename
@config = config
Expand All @@ -96,7 +88,6 @@ def initialize(filename:, config: nil, options: {})
# message - String with the message to log and raise.
#
# Returns nothing.
Contract String => C::None
def fatal_message(message)
Entitlements.logger.fatal(message)
raise RuntimeError, message
Expand All @@ -107,7 +98,6 @@ def fatal_message(message)
# members_in - Optionally a set of Entitlements::Models::Person with the currently calculated member set.
#
# Returns Set[Entitlements::Models::Person] of all matching members.
Contract C::None => C::Or[:calculating, C::SetOf[Entitlements::Models::Person]]
def filtered_members
return :calculating if members == :calculating

Expand All @@ -132,7 +122,6 @@ def filtered_members
# Takes no arguments.
#
# Returns Set[Entitlements::Models::Person] of all matching members.
Contract C::None => C::Or[:calculating, C::SetOf[Entitlements::Models::Person]]
def modified_members
return :calculating if members == :calculating
@modified_members ||= apply_modifiers(members)
Expand All @@ -143,7 +132,6 @@ def modified_members
# members_in - Optionally a set of Entitlements::Models::Person with the currently calculated member set.
#
# Returns Set[Entitlements::Models::Person] of all matching members.
Contract C::None => C::Or[:calculating, C::SetOf[Entitlements::Models::Person]]
def modified_filtered_members
return :calculating if filtered_members == :calculating
@modified_filtered_members ||= apply_modifiers(filtered_members)
Expand All @@ -170,7 +158,6 @@ def filter_applies?(filter_config)
# member_set - Set of Entitlements::Models::Person
#
# Returns a set of Entitlements::Models::Person
Contract C::SetOf[Entitlements::Models::Person] => C::SetOf[Entitlements::Models::Person]
def apply_modifiers(member_set)
result = member_set.dup

Expand Down Expand Up @@ -205,7 +192,6 @@ def apply_modifiers(member_set)
# context - A String (usually a filename) to provide context if there's an error.
#
# Returns true if expired, false if not expired.
Contract C::Or[nil, String], String => C::Or[nil, C::Bool]
def expired?(expiration, context)
return false if Entitlements.config.fetch("ignore_expirations", false)
return false if expiration.nil? || expiration.strip.empty?
Expand All @@ -222,7 +208,6 @@ def expired?(expiration, context)
# rule - A Hash of rules (see "rules" stub below).
#
# Returns Set[Entitlements::Models::Person].
Contract C::HashOf[String => C::Any] => C::Or[:calculating, C::SetOf[Entitlements::Models::Person]]
def members_from_rules(rule)
Entitlements.cache[:calculated] ||= {}
Entitlements.cache[:calculated][rou] ||= {}
Expand Down Expand Up @@ -258,7 +243,6 @@ def members_from_rules(rule)
# rule - A Hash of rules (see "rules" stub below).
#
# Returns Set[Entitlements::Models::Person].
Contract C::HashOf[String => C::Any] => C::SetOf[Entitlements::Models::Person]
def _members_from_rules(rule)
# Empty rule => error.
if rule.keys.empty?
Expand Down Expand Up @@ -302,7 +286,6 @@ def _members_from_rules(rule)
# Takes no arguments.
#
# Returns a Hash.
Contract C::None => C::HashOf[String => C::Any]
def rules
# :nocov:
raise "Must be implemented in child class"
Expand Down Expand Up @@ -371,7 +354,6 @@ def handle_not(rule)
# type - The type.
#
# Returns nothing, but raises an error if the type doesn't match.
Contract String, C::Any, C::Any => nil
def ensure_type!(function, obj, type)
return if obj.is_a?(type)
raise "Invalid type: in #{filename}, expected #{function.inspect} to be a #{type} but got #{obj.inspect}!"
Expand All @@ -382,7 +364,6 @@ def ensure_type!(function, obj, type)
# str - The string that needs to be converted to CamelCase.
#
# Returns a String in CamelCase.
Contract String => String
def camelize(str)
Entitlements::Util::Util.camelize(str)
end
Expand All @@ -392,7 +373,6 @@ def camelize(str)
# Takes no arguments.
#
# Returns a String with the name of the ou.
Contract C::None => String
def ou
File.basename(File.dirname(filename))
end
Expand All @@ -402,7 +382,6 @@ def ou
# Takes no arguments.
#
# Returns a String with the name of the ou.
Contract C::None => String
def rou
File.expand_path(File.dirname(filename)).gsub("#{Entitlements.config_path}/", "").gsub(/^\//, "").gsub(/\//, "/")
end
Expand All @@ -412,7 +391,6 @@ def rou
# Takes no arguments.
#
# Returns a String with the name of the cn.
Contract C::None => String
def cn
File.basename(filename).sub(/\.[^\.]+\z/, "")
end
Expand All @@ -424,7 +402,6 @@ def cn
# Takes no arguments.
#
# Returns a Set with the permitted methods.
Contract C::None => C::SetOf[String]
def allowed_methods
@allowed_methods ||= begin
if config.is_a?(Hash) && config["allowed_methods"]
Expand All @@ -445,7 +422,6 @@ def allowed_methods
# function_in - String with the function name from the definition.
#
# Returns the underlying function name if aliased, or else what was entered.
Contract String => String
def function_for(function_in)
ALIAS_METHODS[function_in] || function_in
end
Expand All @@ -456,7 +432,6 @@ def function_for(function_in)
# Takes no arguments.
#
# Returns an Set of Strings with allowed methods.
Contract C::None => C::SetOf[String]
def whitelisted_methods
Set.new(Entitlements::Data::Groups::Calculated.rules_index.keys)
end
Expand Down
5 changes: 0 additions & 5 deletions lib/entitlements/data/groups/calculated/filters/base.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ class Base
# member - Entitlements::Models::Person object
#
# Return true if the member is to be filtered out, false if the member does not match the filter.
Contract Entitlements::Models::Person => C::Bool
def filtered?(_member)
# :nocov:
raise "Must be implemented in child class"
Expand All @@ -29,10 +28,6 @@ def filtered?(_member)
#
# filter - Either :none, :all, or an array of string conditions passed through to the filter
# config - Configuration data (Hash, optional)
Contract C::KeywordArgs[
filter: C::Or[:none, C::ArrayOf[String]],
config: C::Maybe[Hash]
] => C::Any
def initialize(filter:, config: {})
@filter = filter
@config = config
Expand Down
1 change: 0 additions & 1 deletion lib/entitlements/data/groups/calculated/modifiers/expiration.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ class Expiration < Base
# result - Set of Entitlements::Models::Person (mutated).
#
# Return true if we made any changes, false otherwise.
Contract C::SetOf[Entitlements::Models::Person] => C::Bool
def modify(result)
return false if Entitlements.config.fetch("ignore_expirations", false)
# If group is already empty, we have nothing to consider modifying, regardless
Expand Down
1 change: 0 additions & 1 deletion lib/entitlements/data/groups/calculated/ruby.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ class Ruby < Entitlements::Data::Groups::Calculated::Base
# Takes no arguments.
#
# Returns a Set[Entitlements::Models::Person] with DN's of the people in the group.
Contract C::None => C::SetOf[Entitlements::Models::Person]
def members
@members ||= begin
Entitlements.logger.debug "Calculating members from #{filename}"
Expand Down
5 changes: 0 additions & 5 deletions lib/entitlements/data/groups/calculated/rules/base.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ class Base
# options - Optional hash of additional method-specific options
#
# Returns a Set[Entitlements::Models::Person].
Contract C::KeywordArgs[
value: String,
filename: C::Maybe[String],
options: C::Optional[C::HashOf[Symbol => C::Any]]
] => C::SetOf[Entitlements::Models::Person]
def self.matches(value:, filename: nil, options: {})
# :nocov:
raise "matches() must be defined in the child class #{self.class}!"
Expand Down
6 changes: 0 additions & 6 deletions lib/entitlements/data/groups/calculated/rules/group.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ class Group < Entitlements::Data::Groups::Calculated::Rules::Base
# options - Optional hash of additional method-specific options
#
# Returns a Set[Entitlements::Models::Person].
Contract C::KeywordArgs[
value: String,
filename: C::Maybe[String],
options: C::Optional[C::HashOf[Symbol => C::Any]]
] => C::SetOf[Entitlements::Models::Person]
def self.matches(value:, filename: nil, options: {})
# We've asked for a managed group, so we need to calculate that group and return its members.
# First parse the value into the ou and cn.
Expand Down Expand Up @@ -98,7 +93,6 @@ def self.matches(value:, filename: nil, options: {})
# path - A String with the directory structure relative to Entitlements.config_path
#
# Returns a Set of Hashes with { "file_without_extension" => "extension" }
Contract String, C::KeywordArgs[options: C::HashOf[Symbol => C::Any]] => C::HashOf[String => String]
def self.files_for(path, options:)
@files_for_cache ||= {}
@files_for_cache[path] ||= begin
Expand Down
5 changes: 0 additions & 5 deletions lib/entitlements/data/groups/calculated/rules/username.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ class Username < Entitlements::Data::Groups::Calculated::Rules::Base
# options - Optional hash of additional method-specific options
#
# Returns a Set[Entitlements::Models::Person].
Contract C::KeywordArgs[
value: String,
filename: C::Maybe[String],
options: C::Optional[C::HashOf[Symbol => C::Any]]
] => C::SetOf[Entitlements::Models::Person]
def self.matches(value:, filename: nil, options: {})
# Username is easy - the value is the uid.
begin
Expand Down
Loading
Loading