3scale/porta

View on GitHub
lib/three_scale/tenant_id_integrity_checker.rb

Summary

Maintainability
A
1 hr
Test Coverage

Method can_skip_asociation? has a Cognitive Complexity of 10 (exceeds 5 allowed). Consider refactoring.
Open

    def can_skip_asociation?(association, processed: [])
      # we can ignore these as they can't be automatically excluded but are redundant for the check anyway
      ignored = {
        Service => %i[all_metrics], # all metrics of service and APIs used by service so is redundant
        # only master has provider_accounts and it is normal that all will mismatch
Severity: Minor
Found in lib/three_scale/tenant_id_integrity_checker.rb - About 1 hr to fix

Cognitive Complexity

Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

A method's cognitive complexity is based on a few simple rules:

  • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
  • Code is considered more complex for each "break in the linear flow of the code"
  • Code is considered more complex when "flow breaking structures are nested"

Further reading

ThreeScale::TenantIDIntegrityChecker#can_skip_asociation? refers to 'association' more than self (maybe move it to another class?)
Open

      model = association.active_record

      return true if ignored[model]&.include?(association.name)

      # we live in a perfect world where all associations have an inverse so we can skip polymorphic ones

Feature Envy occurs when a code fragment references another object more often than it references itself, or when several clients do the same series of manipulations on a particular type of object.

Feature Envy reduces the code's ability to communicate intent: code that "belongs" on one class but which is located in another can be hard to find, and may upset the "System of Names" in the host class.

Feature Envy also affects the design's flexibility: A code fragment that is in the wrong class creates couplings that may not be natural within the application's domain, and creates a loss of cohesion in the unwilling host class.

Feature Envy often arises because it must manipulate other objects (usually its arguments) to get them into a useful form, and one force preventing them (the arguments) doing this themselves is that the common knowledge lives outside the arguments, or the arguments are of too basic a type to justify extending that type. Therefore there must be something which 'knows' about the contents or purposes of the arguments. That thing would have to be more than just a basic type, because the basic types are either containers which don't know about their contents, or they are single objects which can't capture their relationship with their fellows of the same type. So, this thing with the extra knowledge should be reified into a class, and the utility method will most likely belong there.

Example

Running Reek on:

class Warehouse
  def sale_price(item)
    (item.price - item.rebate) * @vat
  end
end

would report:

Warehouse#total_price refers to item more than self (FeatureEnvy)

since this:

(item.price - item.rebate)

belongs to the Item class, not the Warehouse.

ThreeScale::TenantIDIntegrityChecker#pluck_pks has 4 parameters
Open

    def pluck_pks(joined_relation, association:, assoc_table:, table:)

A Long Parameter List occurs when a method has a lot of parameters.

Example

Given

class Dummy
  def long_list(foo,bar,baz,fling,flung)
    puts foo,bar,baz,fling,flung
  end
end

Reek would report the following warning:

test.rb -- 1 warning:
  [2]:Dummy#long_list has 5 parameters (LongParameterList)

A common solution to this problem would be the introduction of parameter objects.

ThreeScale::TenantIDIntegrityChecker#can_skip_asociation? has approx 10 statements
Open

    def can_skip_asociation?(association, processed: [])

A method with Too Many Statements is any method that has a large number of lines.

Too Many Statements warns about any method that has more than 5 statements. Reek's smell detector for Too Many Statements counts +1 for every simple statement in a method and +1 for every statement within a control structure (if, else, case, when, for, while, until, begin, rescue) but it doesn't count the control structure itself.

So the following method would score +6 in Reek's statement-counting algorithm:

def parse(arg, argv, &error)
  if !(val = arg) and (argv.empty? or /\A-/ =~ (val = argv[0]))
    return nil, block, nil                                         # +1
  end
  opt = (val = parse_arg(val, &error))[1]                          # +2
  val = conv_arg(*val)                                             # +3
  if opt and !arg
    argv.shift                                                     # +4
  else
    val[0] = nil                                                   # +5
  end
  val                                                              # +6
end

(You might argue that the two assigments within the first @if@ should count as statements, and that perhaps the nested assignment should count as +2.)

ThreeScale::TenantIDIntegrityChecker#inconsistent_pairs_for refers to 'association' more than self (maybe move it to another class?)
Open

      table_alias = last_table_alias_from_sql(model.joins(association.name).to_sql)
      found = model.joins(association.name).where.not("#{table_alias}.tenant_id = #{table}.tenant_id")
      found = found.merge(Account.where(provider: false).or(Account.where(provider: nil))) if model == Account && association.name == :provider_account
      found = pluck_pks(found, association: association, table: table, assoc_table: table_alias)
      found.map { ["#{model}#{_1}", association.name, "#{association.klass}#{_2}"] }

Feature Envy occurs when a code fragment references another object more often than it references itself, or when several clients do the same series of manipulations on a particular type of object.

Feature Envy reduces the code's ability to communicate intent: code that "belongs" on one class but which is located in another can be hard to find, and may upset the "System of Names" in the host class.

Feature Envy also affects the design's flexibility: A code fragment that is in the wrong class creates couplings that may not be natural within the application's domain, and creates a loss of cohesion in the unwilling host class.

Feature Envy often arises because it must manipulate other objects (usually its arguments) to get them into a useful form, and one force preventing them (the arguments) doing this themselves is that the common knowledge lives outside the arguments, or the arguments are of too basic a type to justify extending that type. Therefore there must be something which 'knows' about the contents or purposes of the arguments. That thing would have to be more than just a basic type, because the basic types are either containers which don't know about their contents, or they are single objects which can't capture their relationship with their fellows of the same type. So, this thing with the extra knowledge should be reified into a class, and the utility method will most likely belong there.

Example

Running Reek on:

class Warehouse
  def sale_price(item)
    (item.price - item.rebate) * @vat
  end
end

would report:

Warehouse#total_price refers to item more than self (FeatureEnvy)

since this:

(item.price - item.rebate)

belongs to the Item class, not the Warehouse.

ThreeScale::TenantIDIntegrityChecker#pk_fields refers to 'model' more than self (maybe move it to another class?)
Open

      model.primary_key ? Array(model.primary_key) : model.connection.primary_keys(model.table_name)

Feature Envy occurs when a code fragment references another object more often than it references itself, or when several clients do the same series of manipulations on a particular type of object.

Feature Envy reduces the code's ability to communicate intent: code that "belongs" on one class but which is located in another can be hard to find, and may upset the "System of Names" in the host class.

Feature Envy also affects the design's flexibility: A code fragment that is in the wrong class creates couplings that may not be natural within the application's domain, and creates a loss of cohesion in the unwilling host class.

Feature Envy often arises because it must manipulate other objects (usually its arguments) to get them into a useful form, and one force preventing them (the arguments) doing this themselves is that the common knowledge lives outside the arguments, or the arguments are of too basic a type to justify extending that type. Therefore there must be something which 'knows' about the contents or purposes of the arguments. That thing would have to be more than just a basic type, because the basic types are either containers which don't know about their contents, or they are single objects which can't capture their relationship with their fellows of the same type. So, this thing with the extra knowledge should be reified into a class, and the utility method will most likely belong there.

Example

Running Reek on:

class Warehouse
  def sale_price(item)
    (item.price - item.rebate) * @vat
  end
end

would report:

Warehouse#total_price refers to item more than self (FeatureEnvy)

since this:

(item.price - item.rebate)

belongs to the Item class, not the Warehouse.

ThreeScale::TenantIDIntegrityChecker#inconsistent_pairs_for calls 'association.name' 4 times
Open

      table_alias = last_table_alias_from_sql(model.joins(association.name).to_sql)
      found = model.joins(association.name).where.not("#{table_alias}.tenant_id = #{table}.tenant_id")
      found = found.merge(Account.where(provider: false).or(Account.where(provider: nil))) if model == Account && association.name == :provider_account
      found = pluck_pks(found, association: association, table: table, assoc_table: table_alias)
      found.map { ["#{model}#{_1}", association.name, "#{association.klass}#{_2}"] }

Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

Reek implements a check for Duplicate Method Call.

Example

Here's a very much simplified and contrived example. The following method will report a warning:

def double_thing()
  @other.thing + @other.thing
end

One quick approach to silence Reek would be to refactor the code thus:

def double_thing()
  thing = @other.thing
  thing + thing
end

A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

class Other
  def double_thing()
    thing + thing
  end
end

The approach you take will depend on balancing other factors in your code.

ThreeScale::TenantIDIntegrityChecker#can_skip_asociation? calls 'association.name' 2 times
Open

      return true if ignored[model]&.include?(association.name)

      # we live in a perfect world where all associations have an inverse so we can skip polymorphic ones
      return true if association.polymorphic?

Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

Reek implements a check for Duplicate Method Call.

Example

Here's a very much simplified and contrived example. The following method will report a warning:

def double_thing()
  @other.thing + @other.thing
end

One quick approach to silence Reek would be to refactor the code thus:

def double_thing()
  thing = @other.thing
  thing + thing
end

A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

class Other
  def double_thing()
    thing + thing
  end
end

The approach you take will depend on balancing other factors in your code.

ThreeScale::TenantIDIntegrityChecker#pluck_pks calls 'model_pk.size' 2 times
Open

      res.map { [_1.slice(0, model_pk.size), _1.slice(model_pk.size..-1)] }

Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

Reek implements a check for Duplicate Method Call.

Example

Here's a very much simplified and contrived example. The following method will report a warning:

def double_thing()
  @other.thing + @other.thing
end

One quick approach to silence Reek would be to refactor the code thus:

def double_thing()
  thing = @other.thing
  thing + thing
end

A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

class Other
  def double_thing()
    thing + thing
  end
end

The approach you take will depend on balancing other factors in your code.

ThreeScale::TenantIDIntegrityChecker#inconsistent_pairs_for calls 'model.joins(association.name)' 2 times
Open

      table_alias = last_table_alias_from_sql(model.joins(association.name).to_sql)
      found = model.joins(association.name).where.not("#{table_alias}.tenant_id = #{table}.tenant_id")

Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

Reek implements a check for Duplicate Method Call.

Example

Here's a very much simplified and contrived example. The following method will report a warning:

def double_thing()
  @other.thing + @other.thing
end

One quick approach to silence Reek would be to refactor the code thus:

def double_thing()
  thing = @other.thing
  thing + thing
end

A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

class Other
  def double_thing()
    thing + thing
  end
end

The approach you take will depend on balancing other factors in your code.

ThreeScale::TenantIDIntegrityChecker#can_skip_asociation? calls 'association.inverse_of' 3 times
Open

        return true unless association.inverse_of.polymorphic? || association.inverse_of.scope&.arity&.public_send(:>, 0)
        raise "we can't handle #{association.name} of #{model}"
      end

      return true unless association.klass.attribute_names.include?("tenant_id")

Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

Reek implements a check for Duplicate Method Call.

Example

Here's a very much simplified and contrived example. The following method will report a warning:

def double_thing()
  @other.thing + @other.thing
end

One quick approach to silence Reek would be to refactor the code thus:

def double_thing()
  thing = @other.thing
  thing + thing
end

A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

class Other
  def double_thing()
    thing + thing
  end
end

The approach you take will depend on balancing other factors in your code.

ThreeScale::TenantIDIntegrityChecker#pk_fields calls 'model.primary_key' 2 times
Open

      model.primary_key ? Array(model.primary_key) : model.connection.primary_keys(model.table_name)

Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

Reek implements a check for Duplicate Method Call.

Example

Here's a very much simplified and contrived example. The following method will report a warning:

def double_thing()
  @other.thing + @other.thing
end

One quick approach to silence Reek would be to refactor the code thus:

def double_thing()
  thing = @other.thing
  thing + thing
end

A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

class Other
  def double_thing()
    thing + thing
  end
end

The approach you take will depend on balancing other factors in your code.

Avoid too many return statements within this method.
Open

      return true if association.through_reflection&.try(:klass)&.attribute_names&.include?("tenant_id")
Severity: Major
Found in lib/three_scale/tenant_id_integrity_checker.rb - About 30 mins to fix

    There are no issues that match your filters.

    Category
    Status