Add rubocop with a todo list (#90)

* Add rubocop with a todo list

* Run rubocop as part of the CI workflow

* Autofix for RuboCop: Layout/ElseAlignment

* Regen autogen, removing all claude-scratchpad/*

* Autofix RuboCop Layout/EmptyLine

* AutoFix RuboCop Style/ZeroLengthPredicate

* AutoFix RuboCop Layout/EmptyLinesAroundAccessModifier

* AutoFix RuboCop Layout/EmptyLinesAroundMethodBody

* AutoFix RuboCop Layout/EmptyLinesAroundBlockBody

* AutoFix RuboCop Layout/EmptyLinesAroundClassBody

* AutoFix RuboCop Layout/EndAlignment

* AutoFix RuboCop Layout/ExtraSpacing

* AutoFix RuboCop Layout/FirstHashElementIndentation

* AutoFix RuboCop Layout/HashAlignment

* AutoFix RuboCop Layout/IndentationWidth

* Regenerate todo

* AutoFix RuboCop Layout/SpaceBeforeBlockBraces

* AutoFix RuboCop Layout/SpaceBeforeComma

* AutoFix RuboCop Layout/SpaceInsideBlockBraces

* AutoFix RuboCop Layout/SpaceInsideHashLiteralBraces

* AutoFix RuboCop Layout/TrailingEmptyLines

* Update ci.yml

Co-authored-by: Pieter Oliver <68863060+pieterocp@users.noreply.github.com>

* WIP: Remove claude-scratchpad cruft from rubocop todo

* Specify files explicity in rubocop rake task

* Pin rubocop version to 1.77.0

* AutoFix RuboCop Style/TrailingCommaInHashLiteral

* AutoFix RuboCop Style/StringLiteralsInInterpolation

* AutoFix RuboCop Style/StringLiterals

* AutoFix RuboCop Style/RescueStandardError

* AutoFix RuboCop Style/RedundantPercentQ

* AutoFix RuboCop Style/RedundantConstantBase

* AutoFix RuboCop Style/QuotedSymbols

* AutoFix RuboCop Style/PercentLiteralDelimiters

* AutoFix RuboCop Style/ParallelAssignment

* AutoFix RuboCop Style/MultilineIfModifier

* AutoFix RuboCop Style/Lambda

* AutoFix RuboCop Style/IfUnlessModifier

* AutoFix RuboCop Style/HashSyntax

* AutoFix RuboCop Style/GuardClause

* AutoFix RuboCop Style/ExpandPathArguments

* AutoFix RuboCop Style/BlockDelimiters

* AutoFix RuboCop Style/BlockComments

* AutoFix RuboCop Lint/UnusedMethodArgument

* AutoFix RuboCop Lint/SymbolConversion

* Fix auto-gen formatting + RuboCop AutoFix Style/ModuleFunction

* Code style

---------

Co-authored-by: Pieter Oliver <pieter.oliver@nourishcare.com>
Co-authored-by: Pieter Oliver <68863060+pieterocp@users.noreply.github.com>
This commit is contained in:
Sami Samhuri 2025-07-14 15:20:17 -07:00 committed by GitHub
parent ca80cfef6c
commit 5a9ef5d801
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
26 changed files with 402 additions and 208 deletions

View file

@ -20,4 +20,17 @@ jobs:
- name: Install dependencies - name: Install dependencies
run: bundle install run: bundle install
- name: Run tests - name: Run tests
run: bundle exec rspec spec/ run: bundle exec rake spec
lint:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4
- uses: ruby/setup-ruby@v1
with:
ruby-version: "3.4"
bundler-cache: true
- name: Install dependencies
run: bundle install
- name: Run RuboCop
run: bundle exec rake rubocop

14
.rubocop.yml Normal file
View file

@ -0,0 +1,14 @@
inherit_from: .rubocop_todo.yml
AllCops:
NewCops: enable
Metrics/BlockLength:
CountAsOne: [array, hash, heredoc, method_call]
Metrics/ClassLength:
CountAsOne: [array, hash, heredoc, method_call]
Metrics/MethodLength:
CountAsOne: [array, hash, heredoc, method_call]
Max: 20

160
.rubocop_todo.yml Normal file
View file

@ -0,0 +1,160 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2025-07-10 10:25:51 UTC using RuboCop version 1.77.0.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.
# Offense count: 3
# Configuration parameters: EnforcedStyle, AllowedGems, Include.
# SupportedStyles: Gemfile, gems.rb, gemspec
# Include: **/*.gemspec, **/Gemfile, **/gems.rb
Gemspec/DevelopmentDependencies:
Exclude:
- "grape_logging.gemspec"
# Offense count: 1
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: Severity, Include.
# Include: **/*.gemspec
Gemspec/RequireMFA:
Exclude:
- "grape_logging.gemspec"
# Offense count: 1
# Configuration parameters: Severity, Include.
# Include: **/*.gemspec
Gemspec/RequiredRubyVersion:
Exclude:
- "grape_logging.gemspec"
# Offense count: 1
# Configuration parameters: IgnoreLiteralBranches, IgnoreConstantBranches, IgnoreDuplicateElseBranch.
Lint/DuplicateBranch:
Exclude:
- "lib/grape_logging/util/parameter_filter.rb"
# Offense count: 1
# Configuration parameters: AllowedParentClasses.
Lint/MissingSuper:
Exclude:
- "lib/grape_logging/loggers/filter_parameters.rb"
# Offense count: 3
# Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes.
Metrics/AbcSize:
Max: 38
# Offense count: 8
# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
# AllowedMethods: refine
Metrics/BlockLength:
Max: 90
# Offense count: 3
# Configuration parameters: AllowedMethods, AllowedPatterns.
Metrics/CyclomaticComplexity:
Max: 18
# Offense count: 2
# Configuration parameters: AllowedMethods, AllowedPatterns.
Metrics/PerceivedComplexity:
Max: 19
# Offense count: 2
# Configuration parameters: EnforcedStyle, CheckMethodNames, CheckSymbols, AllowedIdentifiers, AllowedPatterns.
# SupportedStyles: snake_case, normalcase, non_integer
# AllowedIdentifiers: TLS1_1, TLS1_2, capture3, iso8601, rfc1123_date, rfc822, rfc2822, rfc3339, x86_64
Naming/VariableNumber:
Exclude:
- "spec/lib/grape_logging/formatters/rails_spec.rb"
# Offense count: 3
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: MinBranchesCount.
Style/CaseLikeIf:
Exclude:
- "lib/grape_logging/formatters/default.rb"
- "lib/grape_logging/formatters/logstash.rb"
- "lib/grape_logging/formatters/rails.rb"
# Offense count: 17
# Configuration parameters: AllowedConstants.
Style/Documentation:
Enabled: false
# Offense count: 29
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: EnforcedStyle.
# SupportedStyles: always, always_true, never
Style/FrozenStringLiteralComment:
Enabled: false
# Offense count: 1
# This cop supports unsafe autocorrection (--autocorrect-all).
Style/GlobalStdStream:
Exclude:
- "lib/grape_logging/reporters/logger_reporter.rb"
# Offense count: 1
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: EnforcedStyle.
# SupportedStyles: literals, strict
Style/MutableConstant:
Exclude:
- "lib/grape_logging/version.rb"
# Offense count: 10
Style/OpenStructUse:
Exclude:
- "spec/lib/grape_logging/loggers/client_env_spec.rb"
- "spec/lib/grape_logging/loggers/filter_parameters_spec.rb"
- "spec/lib/grape_logging/loggers/request_headers_spec.rb"
- "spec/lib/grape_logging/loggers/response_spec.rb"
# Offense count: 3
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: ConvertCodeThatCanStartToReturnNil, AllowedMethods, MaxChainLength.
# AllowedMethods: present?, blank?, presence, try, try!
Style/SafeNavigation:
Exclude:
- "lib/grape_logging/formatters/rails.rb"
# Offense count: 1
# This cop supports unsafe autocorrection (--autocorrect-all).
Style/SlicingWithRange:
Exclude:
- "lib/grape_logging/loggers/request_headers.rb"
# Offense count: 1
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: RequireEnglish, EnforcedStyle.
# SupportedStyles: use_perl_names, use_english_names, use_builtin_english_names
Style/SpecialGlobalVars:
Exclude:
- "spec/spec_helper.rb"
# Offense count: 3
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: Mode.
Style/StringConcatenation:
Exclude:
- "lib/grape_logging/formatters/json.rb"
- "lib/grape_logging/formatters/lograge.rb"
- "lib/grape_logging/formatters/logstash.rb"
# Offense count: 1
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: AllowMethodsWithArguments, AllowedMethods, AllowedPatterns, AllowComments.
# AllowedMethods: define_method
Style/SymbolProc:
Exclude:
- "lib/grape_logging/util/parameter_filter.rb"
# Offense count: 11
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: AllowHeredoc, AllowURI, AllowQualifiedName, URISchemes, IgnoreCopDirectives, AllowedPatterns, SplitStrings.
# URISchemes: http, https
Layout/LineLength:
Max: 203

View file

@ -1,9 +1,17 @@
require 'bundler/gem_tasks' require 'bundler/gem_tasks'
require 'rspec/core/rake_task' require 'rspec/core/rake_task'
require 'rubocop/rake_task'
RSpec::Core::RakeTask.new(:spec) RSpec::Core::RakeTask.new(:spec) do |spec|
spec.rspec_opts = ['-fd -c']
spec.pattern = FileList['spec/**/*_spec.rb']
end
task default: :spec RuboCop::RakeTask.new(:rubocop) do |t|
t.patterns = ['lib/**/*.rb', 'spec/**/*.rb', 'Rakefile', 'Gemfile', 'grape_logging.gemspec']
end
task default: %i[spec rubocop]
desc 'Release gem and create GitHub release' desc 'Release gem and create GitHub release'
task github_release: :release do task github_release: :release do
@ -14,9 +22,9 @@ task github_release: :release do
# Check if gh CLI is available # Check if gh CLI is available
unless system('which gh > /dev/null 2>&1') unless system('which gh > /dev/null 2>&1')
puts "\n⚠️ GitHub CLI (gh) not found" puts "\n⚠️ GitHub CLI (gh) not found"
puts "To create a GitHub release with auto-generated changelog, install gh:" puts 'To create a GitHub release with auto-generated changelog, install gh:'
puts " brew install gh # macOS with Homebrew" puts ' brew install gh # macOS with Homebrew'
puts " # or visit: https://github.com/cli/cli#installation" puts ' # or visit: https://github.com/cli/cli#installation'
puts "\nYou can manually create the release with:" puts "\nYou can manually create the release with:"
puts " gh release create v#{GrapeLogging::VERSION} --generate-notes" puts " gh release create v#{GrapeLogging::VERSION} --generate-notes"
next next
@ -28,8 +36,8 @@ task github_release: :release do
if system('gh', 'release', 'create', version, '--generate-notes', '--verify-tag') if system('gh', 'release', 'create', version, '--generate-notes', '--verify-tag')
puts "✅ GitHub release #{version} created successfully" puts "✅ GitHub release #{version} created successfully"
else else
puts "❌ Failed to create GitHub release" puts '❌ Failed to create GitHub release'
puts "You can manually create it with:" puts 'You can manually create it with:'
puts " gh release create '#{version}' --generate-notes --verify-tag" puts " gh release create '#{version}' --generate-notes --verify-tag"
end end
end end

View file

@ -1,4 +1,4 @@
lib = File.expand_path('../lib', __FILE__) lib = File.expand_path('lib', __dir__)
$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
require 'grape_logging/version' require 'grape_logging/version'
@ -16,7 +16,7 @@ Gem::Specification.new do |spec|
spec.license = 'MIT' spec.license = 'MIT'
spec.files = `git ls-files -z`.split("\x0").reject { |f| f.match(%r{^(test|spec|features)/}) } spec.files = `git ls-files -z`.split("\x0").reject { |f| f.match(%r{^(test|spec|features)/}) }
spec.bindir = "exe" spec.bindir = 'exe'
spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) }
spec.require_paths = ['lib'] spec.require_paths = ['lib']
@ -25,4 +25,8 @@ Gem::Specification.new do |spec|
spec.add_development_dependency 'rake', '~> 13.3' spec.add_development_dependency 'rake', '~> 13.3'
spec.add_development_dependency 'rspec', '~> 3.5' spec.add_development_dependency 'rspec', '~> 3.5'
# This is pinned to an exact version otherwise we can't know which rules
# are in play at any given time in different environments.
spec.add_development_dependency 'rubocop', '1.77.0'
end end

View file

@ -18,6 +18,7 @@ module GrapeLogging
end end
private private
def format_hash(hash) def format_hash(hash)
hash.keys.sort.map { |key| "#{key}=#{hash[key]}" }.join(' ') hash.keys.sort.map { |key| "#{key}=#{hash[key]}" }.join(' ')
end end

View file

@ -3,9 +3,9 @@ module GrapeLogging
class Logstash class Logstash
def call(severity, datetime, _, data) def call(severity, datetime, _, data)
{ {
:'@timestamp' => datetime.iso8601, '@timestamp': datetime.iso8601,
:'@version' => '1', '@version': '1',
:severity => severity severity: severity
}.merge!(format(data)).to_json + "\n" }.merge!(format(data)).to_json + "\n"
end end

View file

@ -3,7 +3,6 @@ require 'rack/utils'
module GrapeLogging module GrapeLogging
module Formatters module Formatters
class Rails class Rails
def call(severity, datetime, _, data) def call(severity, datetime, _, data)
if data.is_a?(String) if data.is_a?(String)
"#{severity[0..0]} [#{datetime}] #{severity} -- : #{data}\n" "#{severity[0..0]} [#{datetime}] #{severity} -- : #{data}\n"
@ -24,7 +23,7 @@ module GrapeLogging
[ [
"#{exception.message} (#{exception.class})", "#{exception.message} (#{exception.class})",
backtrace_array.join("\n") backtrace_array.join("\n")
].reject{|line| line == ""}.join("\n") ].reject { |line| line == '' }.join("\n")
end end
def format_hash(hash) def format_hash(hash)
@ -32,7 +31,7 @@ module GrapeLogging
# Completed 200 OK in 958ms (Views: 951.1ms | ActiveRecord: 3.8ms) # Completed 200 OK in 958ms (Views: 951.1ms | ActiveRecord: 3.8ms)
# See: actionpack/lib/action_controller/log_subscriber.rb # See: actionpack/lib/action_controller/log_subscriber.rb
message = "" message = ''
additions = [] additions = []
status = hash.delete(:status) status = hash.delete(:status)
params = hash.delete(:params) params = hash.delete(:params)
@ -47,13 +46,12 @@ module GrapeLogging
message << " Parameters: #{params.inspect}\n" if params message << " Parameters: #{params.inspect}\n" if params
message << "Completed #{status} #{::Rack::Utils::HTTP_STATUS_CODES[status]} in #{total_time}ms" message << "Completed #{status} #{::Rack::Utils::HTTP_STATUS_CODES[status]} in #{total_time}ms"
message << " (#{additions.join(" | ".freeze)})" if additions.size > 0 message << " (#{additions.join(' | '.freeze)})" unless additions.empty?
message << "\n" message << "\n"
message << "\n" if defined?(::Rails.env) && ::Rails.env.development? message << "\n" if defined?(::Rails.env) && ::Rails.env.development?
message message
end end
end end
end end
end end

View file

@ -1,7 +1,7 @@
module GrapeLogging module GrapeLogging
module Loggers module Loggers
class Base class Base
def parameters(request, response) def parameters(_request, _response)
{} {}
end end
end end

View file

@ -2,7 +2,7 @@ module GrapeLogging
module Loggers module Loggers
class ClientEnv < GrapeLogging::Loggers::Base class ClientEnv < GrapeLogging::Loggers::Base
def parameters(request, _) def parameters(request, _)
{ ip: request.env["HTTP_X_FORWARDED_FOR"] || request.env["REMOTE_ADDR"], ua: request.env["HTTP_USER_AGENT"] } { ip: request.env['HTTP_X_FORWARDED_FOR'] || request.env['REMOTE_ADDR'], ua: request.env['HTTP_USER_AGENT'] }
end end
end end
end end

View file

@ -3,7 +3,7 @@ module GrapeLogging
class FilterParameters < GrapeLogging::Loggers::Base class FilterParameters < GrapeLogging::Loggers::Base
AD_PARAMS = 'action_dispatch.request.parameters'.freeze AD_PARAMS = 'action_dispatch.request.parameters'.freeze
def initialize(filter_parameters = nil, replacement = nil, exceptions = %w(controller action format)) def initialize(filter_parameters = nil, replacement = nil, exceptions = %w[controller action format])
@filter_parameters = filter_parameters || (defined?(::Rails.application) ? ::Rails.application.config.filter_parameters : []) @filter_parameters = filter_parameters || (defined?(::Rails.application) ? ::Rails.application.config.filter_parameters : [])
@replacement = replacement || '[FILTERED]' @replacement = replacement || '[FILTERED]'
@exceptions = exceptions @exceptions = exceptions
@ -30,8 +30,8 @@ module GrapeLogging
def clean_parameters(parameters) def clean_parameters(parameters)
original_encoding_map = build_encoding_map(parameters) original_encoding_map = build_encoding_map(parameters)
params = transform_key_encoding(parameters, Hash.new{ |h, _| [Encoding::ASCII_8BIT, h] }) params = transform_key_encoding(parameters, Hash.new { |h, _| [Encoding::ASCII_8BIT, h] })
cleaned_params = parameter_filter.filter(params).reject{ |key, _value| @exceptions.include?(key) } cleaned_params = parameter_filter.filter(params).reject { |key, _value| @exceptions.include?(key) }
transform_key_encoding(cleaned_params, original_encoding_map) transform_key_encoding(cleaned_params, original_encoding_map)
end end

View file

@ -1,7 +1,6 @@
module GrapeLogging module GrapeLogging
module Loggers module Loggers
class RequestHeaders < GrapeLogging::Loggers::Base class RequestHeaders < GrapeLogging::Loggers::Base
HTTP_PREFIX = 'HTTP_'.freeze HTTP_PREFIX = 'HTTP_'.freeze
def parameters(request, _) def parameters(request, _)
@ -16,7 +15,6 @@ module GrapeLogging
{ headers: headers } { headers: headers }
end end
end end
end end
end end

View file

@ -11,12 +11,11 @@ module GrapeLogging
# For example, if you POST on a PUT endpoint, response.body is egal to """". # For example, if you POST on a PUT endpoint, response.body is egal to """".
# It's strange, but it's the Grape behavior... # It's strange, but it's the Grape behavior...
def serialized_response_body(response) def serialized_response_body(response)
if response.respond_to?(:body) if response.respond_to?(:body)
# Rack responses # Rack responses
begin begin
response.body.map{ |body| JSON.parse(body.to_s) } response.body.map { |body| JSON.parse(body.to_s) }
rescue # No reason to have "=> e" here when we don't use it.. rescue StandardError # No reason to have "=> e" here when we don't use it..
response.body response.body
end end
else else

View file

@ -3,11 +3,12 @@ require 'grape'
module GrapeLogging module GrapeLogging
module Middleware module Middleware
class RequestLogger < Grape::Middleware::Base class RequestLogger < Grape::Middleware::Base
if defined?(ActiveRecord)
ActiveSupport::Notifications.subscribe('sql.active_record') do |*args| ActiveSupport::Notifications.subscribe('sql.active_record') do |*args|
event = ActiveSupport::Notifications::Event.new(*args) event = ActiveSupport::Notifications::Event.new(*args)
GrapeLogging::Timings.append_db_runtime(event) GrapeLogging::Timings.append_db_runtime(event)
end if defined?(ActiveRecord) end
end
# Persist response status & response (body) # Persist response status & response (body)
# to use int in parameters # to use int in parameters
@ -17,11 +18,12 @@ module GrapeLogging
super super
@included_loggers = @options[:include] || [] @included_loggers = @options[:include] || []
@reporter = if options[:instrumentation_key] @reporter =
Reporters::ActiveSupportReporter.new(@options[:instrumentation_key]) if options[:instrumentation_key]
else Reporters::ActiveSupportReporter.new(@options[:instrumentation_key])
Reporters::LoggerReporter.new(@options[:logger], @options[:formatter], @options[:log_level]) else
end Reporters::LoggerReporter.new(@options[:logger], @options[:formatter], @options[:log_level])
end
end end
def before def before
@ -99,7 +101,7 @@ module GrapeLogging
path: request.path, path: request.path,
params: request.params, params: request.params,
host: request.host, host: request.host,
request_id: env['action_dispatch.request_id'], request_id: env['action_dispatch.request_id']
} }
end end

View file

@ -5,11 +5,11 @@ module GrapeLogging
end end
def write(*args) def write(*args)
@targets.each {|t| t.write(*args)} @targets.each { |t| t.write(*args) }
end end
def close def close
@targets.each(&:close) @targets.each(&:close)
end end
end end
end end

View file

@ -8,4 +8,4 @@ module Reporters
ActiveSupport::Notifications.instrument @instrumentation_key, params ActiveSupport::Notifications.instrument @instrumentation_key, params
end end
end end
end end

View file

@ -3,9 +3,7 @@ module Reporters
def initialize(logger, formatter, log_level) def initialize(logger, formatter, log_level)
@logger = logger.clone || Logger.new(STDOUT) @logger = logger.clone || Logger.new(STDOUT)
@log_level = log_level || :info @log_level = log_level || :info
if @logger.respond_to?(:formatter=) @logger.formatter = formatter || @logger.formatter || GrapeLogging::Formatters::Default.new if @logger.respond_to?(:formatter=)
@logger.formatter = formatter || @logger.formatter || GrapeLogging::Formatters::Default.new
end
end end
def perform(params) def perform(params)

View file

@ -1,21 +1,19 @@
module GrapeLogging module GrapeLogging
module Timings module Timings
extend self def self.db_runtime=(value)
def db_runtime=(value)
Thread.current[:grape_db_runtime] = value Thread.current[:grape_db_runtime] = value
end end
def db_runtime def self.db_runtime
Thread.current[:grape_db_runtime] ||= 0 Thread.current[:grape_db_runtime] ||= 0
end end
def reset_db_runtime def self.reset_db_runtime
self.db_runtime = 0 self.db_runtime = 0
end end
def append_db_runtime(event) def self.append_db_runtime(event)
self.db_runtime += event.duration self.db_runtime += event.duration
end end
end end
end end

View file

@ -1,4 +1,4 @@
if defined?(::Rails.application) if defined?(Rails.application)
if Gem::Version.new(Rails.version) < Gem::Version.new('6.0.0') if Gem::Version.new(Rails.version) < Gem::Version.new('6.0.0')
class ParameterFilter < ActionDispatch::Http::ParameterFilter class ParameterFilter < ActionDispatch::Http::ParameterFilter
def initialize(_replacement, filter_parameters) def initialize(_replacement, filter_parameters)
@ -6,7 +6,7 @@ if defined?(::Rails.application)
end end
end end
else else
require "active_support/parameter_filter" require 'active_support/parameter_filter'
class ParameterFilter < ActiveSupport::ParameterFilter class ParameterFilter < ActiveSupport::ParameterFilter
def initialize(_replacement, filter_parameters) def initialize(_replacement, filter_parameters)
@ -37,9 +37,11 @@ else
class CompiledFilter # :nodoc: class CompiledFilter # :nodoc:
def self.compile(replacement, filters) def self.compile(replacement, filters)
return lambda { |params| params.dup } if filters.empty? return ->(params) { params.dup } if filters.empty?
strings, regexps, blocks = [], [], [] strings = []
regexps = []
blocks = []
filters.each do |item| filters.each do |item|
case item case item
@ -52,8 +54,8 @@ else
end end
end end
deep_regexps, regexps = regexps.partition { |r| r.to_s.include?("\\.".freeze) } deep_regexps, regexps = regexps.partition { |r| r.to_s.include?('\\.'.freeze) }
deep_strings, strings = strings.partition { |s| s.include?("\\.".freeze) } deep_strings, strings = strings.partition { |s| s.include?('\\.'.freeze) }
regexps << Regexp.new(strings.join('|'.freeze), true) unless strings.empty? regexps << Regexp.new(strings.join('|'.freeze), true) unless strings.empty?
deep_regexps << Regexp.new(deep_strings.join('|'.freeze), true) unless deep_strings.empty? deep_regexps << Regexp.new(deep_strings.join('|'.freeze), true) unless deep_strings.empty?
@ -67,7 +69,7 @@ else
@replacement = replacement @replacement = replacement
@regexps = regexps @regexps = regexps
@deep_regexps = deep_regexps.any? ? deep_regexps : nil @deep_regexps = deep_regexps.any? ? deep_regexps : nil
@blocks = blocks @blocks = blocks
end end
def call(original_params, parents = []) def call(original_params, parents = [])

View file

@ -2,11 +2,11 @@ require 'spec_helper'
describe GrapeLogging::Formatters::Rails do describe GrapeLogging::Formatters::Rails do
let(:formatter) { described_class.new } let(:formatter) { described_class.new }
let(:severity) { "INFO" } let(:severity) { 'INFO' }
let(:datetime) { Time.new('2018', '03', '02', '10', '35', '04', '+13:00') } let(:datetime) { Time.new('2018', '03', '02', '10', '35', '04', '+13:00') }
let(:exception_data) { ArgumentError.new('Message') } let(:exception_data) { ArgumentError.new('Message') }
let(:hash_data) { let(:hash_data) do
{ {
status: 200, status: 200,
time: { time: {
@ -14,11 +14,11 @@ describe GrapeLogging::Formatters::Rails do
db: 40.63, db: 40.63,
view: 231.76999999999998 view: 231.76999999999998
}, },
method: "GET", method: 'GET',
path: "/api/endpoint", path: '/api/endpoint',
host: "localhost" host: 'localhost'
} }
} end
describe '#call' do describe '#call' do
context 'string data' do context 'string data' do
@ -36,7 +36,7 @@ describe GrapeLogging::Formatters::Rails do
message = formatter.call(severity, datetime, nil, exception_data) message = formatter.call(severity, datetime, nil, exception_data)
lines = message.split("\n") lines = message.split("\n")
expect(lines[0]).to eq "I [2018-03-02 10:35:04 +1300] INFO -- : Message (ArgumentError)" expect(lines[0]).to eq 'I [2018-03-02 10:35:04 +1300] INFO -- : Message (ArgumentError)'
expect(lines[1]).to include '.rb' expect(lines[1]).to include '.rb'
expect(lines.size).to be > 1 expect(lines.size).to be > 1
end end
@ -52,9 +52,9 @@ describe GrapeLogging::Formatters::Rails do
it 'includes params if included (from GrapeLogging::Loggers::FilterParameters)' do it 'includes params if included (from GrapeLogging::Loggers::FilterParameters)' do
hash_data.merge!( hash_data.merge!(
params: { params: {
"some_param" => { 'some_param' => {
value_1: "123", value_1: '123',
value_2: "456" value_2: '456'
} }
} }
) )
@ -62,17 +62,18 @@ describe GrapeLogging::Formatters::Rails do
message = formatter.call(severity, datetime, nil, hash_data) message = formatter.call(severity, datetime, nil, hash_data)
lines = message.split("\n") lines = message.split("\n")
expected_output = if RUBY_VERSION >= '3.4' expected_output =
' Parameters: {"some_param" => {value_1: "123", value_2: "456"}}' if RUBY_VERSION >= '3.4'
else ' Parameters: {"some_param" => {value_1: "123", value_2: "456"}}'
' Parameters: {"some_param"=>{:value_1=>"123", :value_2=>"456"}}' else
end ' Parameters: {"some_param"=>{:value_1=>"123", :value_2=>"456"}}'
end
expect(lines.first).to eq expected_output expect(lines.first).to eq expected_output
expect(lines.last).to eq "Completed 200 OK in 272.4ms (Views: 231.77ms | DB: 40.63ms)" expect(lines.last).to eq 'Completed 200 OK in 272.4ms (Views: 231.77ms | DB: 40.63ms)'
end end
end end
context "unhandled data" do context 'unhandled data' do
it 'returns the #inspect string representation' do it 'returns the #inspect string representation' do
message = formatter.call(severity, datetime, nil, [1, 2, 3]) message = formatter.call(severity, datetime, nil, [1, 2, 3])
@ -80,5 +81,4 @@ describe GrapeLogging::Formatters::Rails do
end end
end end
end end
end end

View file

@ -4,14 +4,14 @@ require 'ostruct'
describe GrapeLogging::Loggers::ClientEnv do describe GrapeLogging::Loggers::ClientEnv do
let(:ip) { '10.0.0.1' } let(:ip) { '10.0.0.1' }
let(:user_agent) { 'user agent' } let(:user_agent) { 'user agent' }
let(:forwarded_for) { "forwarded for" } let(:forwarded_for) { 'forwarded for' }
let(:remote_addr) { "remote address" } let(:remote_addr) { 'remote address' }
context 'forwarded for' do context 'forwarded for' do
let(:mock_request) do let(:mock_request) do
OpenStruct.new(env: { OpenStruct.new(env: {
"HTTP_X_FORWARDED_FOR" => forwarded_for 'HTTP_X_FORWARDED_FOR' => forwarded_for
}) })
end end
it 'sets the ip key' do it 'sets the ip key' do
@ -27,8 +27,8 @@ describe GrapeLogging::Loggers::ClientEnv do
context 'remote address' do context 'remote address' do
let(:mock_request) do let(:mock_request) do
OpenStruct.new(env: { OpenStruct.new(env: {
"REMOTE_ADDR" => remote_addr 'REMOTE_ADDR' => remote_addr
}) })
end end
it 'sets the ip key' do it 'sets the ip key' do
@ -39,8 +39,8 @@ describe GrapeLogging::Loggers::ClientEnv do
context 'user agent' do context 'user agent' do
let(:mock_request) do let(:mock_request) do
OpenStruct.new(env: { OpenStruct.new(env: {
"HTTP_USER_AGENT" => user_agent 'HTTP_USER_AGENT' => user_agent
}) })
end end
it 'sets the ua key' do it 'sets the ua key' do

View file

@ -1,27 +1,26 @@
require 'spec_helper' require 'spec_helper'
require 'ostruct' require 'ostruct'
describe GrapeLogging::Loggers::FilterParameters do describe GrapeLogging::Loggers::FilterParameters do
let(:filtered_parameters) { %w[one four] } let(:filtered_parameters) { %w[one four] }
let(:mock_request) do let(:mock_request) do
OpenStruct.new(params: { OpenStruct.new(params: {
'this_one' => 'this one', 'this_one' => 'this one',
'that_one' => 'one', 'that_one' => 'one',
'two' => 'two', 'two' => 'two',
'three' => 'three', 'three' => 'three',
'four' => 'four', 'four' => 'four',
"\xff" => 'invalid utf8', "\xff" => 'invalid utf8'
}) })
end end
let(:mock_request_with_deep_nesting) do let(:mock_request_with_deep_nesting) do
deep_clone = lambda { Marshal.load Marshal.dump mock_request.params } deep_clone = -> { Marshal.load Marshal.dump mock_request.params }
OpenStruct.new( OpenStruct.new(
params: deep_clone.call.merge( params: deep_clone.call.merge(
'five' => deep_clone.call.merge( 'five' => deep_clone.call.merge(
deep_clone.call.merge({'six' => {'seven' => 'seven', 'eight' => 'eight', 'one' => 'another one'}}) deep_clone.call.merge({ 'six' => { 'seven' => 'seven', 'eight' => 'eight', 'one' => 'another one' } })
) )
) )
) )
@ -36,37 +35,37 @@ describe GrapeLogging::Loggers::FilterParameters do
shared_examples 'filtering' do shared_examples 'filtering' do
it 'filters out sensitive parameters' do it 'filters out sensitive parameters' do
expect(subject.parameters(mock_request, nil)).to eq(params: { expect(subject.parameters(mock_request, nil)).to eq(params: {
'this_one' => subject.instance_variable_get('@replacement'), 'this_one' => subject.instance_variable_get('@replacement'),
'that_one' => subject.instance_variable_get('@replacement'), 'that_one' => subject.instance_variable_get('@replacement'),
'two' => 'two', 'two' => 'two',
'three' => 'three', 'three' => 'three',
'four' => subject.instance_variable_get('@replacement'), 'four' => subject.instance_variable_get('@replacement'),
"\xff" => 'invalid utf8', "\xff" => 'invalid utf8'
}) })
end end
it 'deeply filters out sensitive parameters' do it 'deeply filters out sensitive parameters' do
expect(subject.parameters(mock_request_with_deep_nesting, nil)).to eq(params: { expect(subject.parameters(mock_request_with_deep_nesting, nil)).to eq(params: {
'this_one' => subject.instance_variable_get('@replacement'), 'this_one' => subject.instance_variable_get('@replacement'),
'that_one' => subject.instance_variable_get('@replacement'), 'that_one' => subject.instance_variable_get('@replacement'),
'two' => 'two', 'two' => 'two',
'three' => 'three', 'three' => 'three',
'four' => subject.instance_variable_get('@replacement'), 'four' => subject.instance_variable_get('@replacement'),
"\xff" => 'invalid utf8', "\xff" => 'invalid utf8',
'five' => { 'five' => {
'this_one' => subject.instance_variable_get('@replacement'), 'this_one' => subject.instance_variable_get('@replacement'),
'that_one' => subject.instance_variable_get('@replacement'), 'that_one' => subject.instance_variable_get('@replacement'),
'two' => 'two', 'two' => 'two',
'three' => 'three', 'three' => 'three',
'four' => subject.instance_variable_get('@replacement'), 'four' => subject.instance_variable_get('@replacement'),
"\xff" => 'invalid utf8', "\xff" => 'invalid utf8',
'six' => { 'six' => {
'seven' => 'seven', 'seven' => 'seven',
'eight' => 'eight', 'eight' => 'eight',
'one' => subject.instance_variable_get('@replacement'), 'one' => subject.instance_variable_get('@replacement')
}, }
}, }
}) })
end end
end end
@ -84,8 +83,8 @@ describe GrapeLogging::Loggers::FilterParameters do
it 'converts keys to strings' do it 'converts keys to strings' do
expect(subject.parameters(mock_request, nil)).to eq(params: { expect(subject.parameters(mock_request, nil)).to eq(params: {
'sneaky_symbol' => 'hey!', 'sneaky_symbol' => 'hey!'
}) })
end end
end end
end end

View file

@ -3,38 +3,44 @@ require 'ostruct'
describe GrapeLogging::Loggers::RequestHeaders do describe GrapeLogging::Loggers::RequestHeaders do
let(:mock_request) do let(:mock_request) do
OpenStruct.new(env: {HTTP_REFERER: 'http://example.com', HTTP_ACCEPT: 'text/plain'}) OpenStruct.new(env: { HTTP_REFERER: 'http://example.com', HTTP_ACCEPT: 'text/plain' })
end end
let(:mock_request_with_unhandled_headers) do let(:mock_request_with_unhandled_headers) do
OpenStruct.new(env: { OpenStruct.new(env: {
HTTP_REFERER: 'http://example.com', HTTP_REFERER: 'http://example.com',
"PATH_INFO"=>"/api/v1/users" 'PATH_INFO' => '/api/v1/users'
}) })
end end
let(:mock_request_with_long_headers) do let(:mock_request_with_long_headers) do
OpenStruct.new(env: { OpenStruct.new(env: {
HTTP_REFERER: 'http://example.com', HTTP_REFERER: 'http://example.com',
HTTP_USER_AGENT: "Mozilla/5.0" HTTP_USER_AGENT: 'Mozilla/5.0'
}) })
end end
it 'strips HTTP_ from the parameter' do it 'strips HTTP_ from the parameter' do
expect(subject.parameters(mock_request, nil)).to eq({ expect(subject.parameters(mock_request, nil)).to eq(
headers: {'Referer' => 'http://example.com', 'Accept' => 'text/plain'} {
}) headers: { 'Referer' => 'http://example.com', 'Accept' => 'text/plain' }
}
)
end end
it 'only handle things which start with HTTP_' do it 'only handle things which start with HTTP_' do
expect(subject.parameters(mock_request_with_unhandled_headers, nil)).to eq({ expect(subject.parameters(mock_request_with_unhandled_headers, nil)).to eq(
headers: {'Referer' => 'http://example.com' } {
}) headers: { 'Referer' => 'http://example.com' }
}
)
end end
it 'substitutes _ with -' do it 'substitutes _ with -' do
expect(subject.parameters(mock_request_with_long_headers, nil)).to eq({ expect(subject.parameters(mock_request_with_long_headers, nil)).to eq(
headers: {'Referer' => 'http://example.com', 'User-Agent' => 'Mozilla/5.0' } {
}) headers: { 'Referer' => 'http://example.com', 'User-Agent' => 'Mozilla/5.0' }
}
)
end end
end end

View file

@ -4,25 +4,21 @@ require 'ostruct'
describe GrapeLogging::Loggers::Response do describe GrapeLogging::Loggers::Response do
context 'with a parseable JSON body' do context 'with a parseable JSON body' do
let(:response) do let(:response) do
OpenStruct.new(body: [{"one": "two", "three": {"four": 5}}]) OpenStruct.new(body: [{ one: 'two', three: { four: 5 } }])
end end
it 'returns an array of parsed JSON objects' do it 'returns an array of parsed JSON objects' do
expect(subject.parameters(nil, response)).to eq({ expect(subject.parameters(nil, response)).to eq({ response: [response.body.first] })
response: [response.body.first],
})
end end
end end
context 'with a body that is not parseable JSON' do context 'with a body that is not parseable JSON' do
let(:response) do let(:response) do
OpenStruct.new(body: "this is a body") OpenStruct.new(body: 'this is a body')
end end
it 'just returns the body' do it 'just returns the body' do
expect(subject.parameters(nil, response)).to eq({ expect(subject.parameters(nil, response)).to eq({ response: response.body })
response: response.body,
})
end end
end end
end end

View file

@ -4,10 +4,10 @@ require 'rack'
describe GrapeLogging::Middleware::RequestLogger do describe GrapeLogging::Middleware::RequestLogger do
let(:env) { { 'action_dispatch.request_id' => 'request-abc123' } } let(:env) { { 'action_dispatch.request_id' => 'request-abc123' } }
let(:subject) { request.send(request_method, path, env) } let(:subject) { request.send(request_method, path, env) }
let(:app) { proc{ [status, {} , ['response body']] } } let(:app) { proc { [status, {}, ['response body']] } }
let(:stack) { described_class.new app, options } let(:stack) { described_class.new app, options }
let(:request) { Rack::MockRequest.new(stack) } let(:request) { Rack::MockRequest.new(stack) }
let(:options) { {include: [], logger: logger} } let(:options) { { include: [], logger: logger } }
let(:logger) { double('logger') } let(:logger) { double('logger') }
let(:path) { '/' } let(:path) { '/' }
let(:request_method) { 'get' } let(:request_method) { 'get' }
@ -51,7 +51,7 @@ describe GrapeLogging::Middleware::RequestLogger do
end end
context 'with a nil response' do context 'with a nil response' do
let(:app) { proc{ [500, {} , nil] } } let(:app) { proc { [500, {}, nil] } }
it 'should log "fail" instead of a status' do it 'should log "fail" instead of a status' do
expect(Rack::MockResponse).to receive(:new) { nil } expect(Rack::MockResponse).to receive(:new) { nil }
expect(logger).to receive('info') do |arguments| expect(logger).to receive('info') do |arguments|
@ -66,7 +66,7 @@ describe GrapeLogging::Middleware::RequestLogger do
options[:include] << GrapeLogging::Loggers::RequestHeaders.new options[:include] << GrapeLogging::Loggers::RequestHeaders.new
options[:include] << GrapeLogging::Loggers::ClientEnv.new options[:include] << GrapeLogging::Loggers::ClientEnv.new
options[:include] << GrapeLogging::Loggers::Response.new options[:include] << GrapeLogging::Loggers::Response.new
options[:include] << GrapeLogging::Loggers::FilterParameters.new(["replace_me"]) options[:include] << GrapeLogging::Loggers::FilterParameters.new(['replace_me'])
end end
%w[get put post delete options head patch].each do |the_method| %w[get put post delete options head patch].each do |the_method|
@ -86,9 +86,9 @@ describe GrapeLogging::Middleware::RequestLogger do
it 'should filter parameters in the log' do it 'should filter parameters in the log' do
expect(logger).to receive('info') do |arguments| expect(logger).to receive('info') do |arguments|
expect(arguments[:params]).to eq( expect(arguments[:params]).to eq(
"replace_me" => '[FILTERED]', 'replace_me' => '[FILTERED]',
"replace_me_too" => '[FILTERED]', 'replace_me_too' => '[FILTERED]',
"cant_touch_this" => 'should see' 'cant_touch_this' => 'should see'
) )
end end
parameters = { parameters = {

View file

@ -33,57 +33,55 @@ RSpec.configure do |config|
# triggering implicit auto-inclusion in groups with matching metadata. # triggering implicit auto-inclusion in groups with matching metadata.
config.shared_context_metadata_behavior = :apply_to_host_groups config.shared_context_metadata_behavior = :apply_to_host_groups
# The settings below are suggested to provide a good initial experience # The settings below are suggested to provide a good initial experience
# with RSpec, but feel free to customize to your heart's content. # with RSpec, but feel free to customize to your heart's content.
=begin # # This allows you to limit a spec run to individual examples or groups
# This allows you to limit a spec run to individual examples or groups # # you care about by tagging them with `:focus` metadata. When nothing
# you care about by tagging them with `:focus` metadata. When nothing # # is tagged with `:focus`, all examples get run. RSpec also provides
# is tagged with `:focus`, all examples get run. RSpec also provides # # aliases for `it`, `describe`, and `context` that include `:focus`
# aliases for `it`, `describe`, and `context` that include `:focus` # # metadata: `fit`, `fdescribe` and `fcontext`, respectively.
# metadata: `fit`, `fdescribe` and `fcontext`, respectively. # config.filter_run_when_matching :focus
config.filter_run_when_matching :focus #
# # Allows RSpec to persist some state between runs in order to support
# Allows RSpec to persist some state between runs in order to support # # the `--only-failures` and `--next-failure` CLI options. We recommend
# the `--only-failures` and `--next-failure` CLI options. We recommend # # you configure your source control system to ignore this file.
# you configure your source control system to ignore this file. # config.example_status_persistence_file_path = "spec/examples.txt"
config.example_status_persistence_file_path = "spec/examples.txt" #
# # Limits the available syntax to the non-monkey patched syntax that is
# Limits the available syntax to the non-monkey patched syntax that is # # recommended. For more details, see:
# recommended. For more details, see: # # - http://rspec.info/blog/2012/06/rspecs-new-expectation-syntax/
# - http://rspec.info/blog/2012/06/rspecs-new-expectation-syntax/ # # - http://www.teaisaweso.me/blog/2013/05/27/rspecs-new-message-expectation-syntax/
# - http://www.teaisaweso.me/blog/2013/05/27/rspecs-new-message-expectation-syntax/ # # - http://rspec.info/blog/2014/05/notable-changes-in-rspec-3/#zero-monkey-patching-mode
# - http://rspec.info/blog/2014/05/notable-changes-in-rspec-3/#zero-monkey-patching-mode # config.disable_monkey_patching!
config.disable_monkey_patching! #
# # This setting enables warnings. It's recommended, but in some cases may
# This setting enables warnings. It's recommended, but in some cases may # # be too noisy due to issues in dependencies.
# be too noisy due to issues in dependencies. # config.warnings = true
config.warnings = true #
# # Many RSpec users commonly either run the entire suite or an individual
# Many RSpec users commonly either run the entire suite or an individual # # file, and it's useful to allow more verbose output when running an
# file, and it's useful to allow more verbose output when running an # # individual spec file.
# individual spec file. # if config.files_to_run.one?
if config.files_to_run.one? # # Use the documentation formatter for detailed output,
# Use the documentation formatter for detailed output, # # unless a formatter has already been configured
# unless a formatter has already been configured # # (e.g. via a command-line flag).
# (e.g. via a command-line flag). # config.default_formatter = 'doc'
config.default_formatter = 'doc' # end
end #
# # Print the 10 slowest examples and example groups at the
# Print the 10 slowest examples and example groups at the # # end of the spec run, to help surface which specs are running
# end of the spec run, to help surface which specs are running # # particularly slow.
# particularly slow. # config.profile_examples = 10
config.profile_examples = 10 #
# # Run specs in random order to surface order dependencies. If you find an
# Run specs in random order to surface order dependencies. If you find an # # order dependency and want to debug it, you can fix the order by providing
# order dependency and want to debug it, you can fix the order by providing # # the seed, which is printed after each run.
# the seed, which is printed after each run. # # --seed 1234
# --seed 1234 # config.order = :random
config.order = :random #
# # Seed global randomization in this process using the `--seed` CLI option.
# Seed global randomization in this process using the `--seed` CLI option. # # Setting this allows you to use `--seed` to deterministically reproduce
# Setting this allows you to use `--seed` to deterministically reproduce # # test failures related to randomization by passing the same `--seed` value
# test failures related to randomization by passing the same `--seed` value # # as the one that triggered the failure.
# as the one that triggered the failure. # Kernel.srand config.seed
Kernel.srand config.seed
=end
end end