Skip to content

Commit

Permalink
feat: use SemanticLogger for Padrino (#662)
Browse files Browse the repository at this point in the history
  • Loading branch information
bethesque authored Jan 23, 2024
1 parent b950e50 commit 5d9d700
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 19 deletions.
5 changes: 0 additions & 5 deletions lib/pact_broker/app.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
# Must be defined before loading Padrino
PADRINO_LOGGER ||= {
ENV.fetch("RACK_ENV", "production").to_sym => { log_level: :error, stream: :stdout, format_datetime: "%Y-%m-%dT%H:%M:%S.000%:z" }
}

require "pact_broker/configuration"
require "pact_broker/db"
require "pact_broker/initializers/database_connection"
Expand Down
29 changes: 20 additions & 9 deletions lib/pact_broker/ui.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,25 @@
require "pact_broker/configuration"
# Must be defined before loading Padrino
# Stop Padrino creating a log file, as it will try to create it in the gems directory
# http://www.padrinorb.com/api/Padrino/Logger.html
unless defined? PADRINO_LOGGER
log_path = File.join(PactBroker.configuration.log_dir, "ui.log")
PADRINO_LOGGER = {
production: { log_level: :error, stream: :to_file, log_path: log_path },
staging: { log_level: :error, stream: :to_file, log_path: log_path },
test: { log_level: :warn, stream: :to_file, log_path: log_path },
development: { log_level: :warn, stream: :to_file, log_path: log_path }
}
# This configuration will be replaced by the SemanticLogger later on.
PADRINO_LOGGER ||= {
ENV.fetch("RACK_ENV", "production").to_sym => { stream: :stdout }
}

require "padrino-core"

class PactBrokerPadrinoLogger < SemanticLogger::Logger
include Padrino::Logger::Extensions

# Padrino expects level to return an integer, not a symbol
def level
Padrino::Logger::Levels[SemanticLogger.default_level]
end
end

Padrino.logger = PactBrokerPadrinoLogger.new("Padrino")
# Log a test message to ensure that the logger works properly, as it only
# seems to be used in production.
Padrino.logger.info("Padrino has been configured with SemanticLogger")

require "pact_broker/ui/app"
6 changes: 4 additions & 2 deletions lib/pact_broker/ui/controllers/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ class Base < Padrino::Application
using PactBroker::StringRefinements

set :root, File.join(File.dirname(__FILE__), "..")
set :show_exceptions, ENV["RACK_ENV"] != "production"
set :dump_errors, false # The padrino logger logs these for us. If this is enabled we get duplicate logging.
set :show_exceptions, ENV["RACK_ENV"] == "development"
# The padrino logger logs these for us, but only in production. If this is enabled we get duplicate logging.
set :dump_errors, ENV["RACK_ENV"] != "production"
set :raise_errors, ENV["RACK_ENV"] == "test"

def base_url
# Using the X-Forwarded headers in the UI can leave the app vulnerable
Expand Down
6 changes: 3 additions & 3 deletions spec/lib/pact_broker/ui/controllers/index_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,22 @@ module Controllers

context "when pagination parameters are present" do
it "passes through pagination parameters to the search" do
expect(PactBroker::Index::Service).to receive(:find_index_items).with(hash_including(page_number: 2, page_size: 40))
expect(PactBroker::Index::Service).to receive(:find_index_items).with(hash_including(page_number: 2, page_size: 40)).and_call_original
get "/", { page: "2", pageSize: "40" }
end
end

context "when pagination parameters are not present" do
context "when tags=true" do
it "passes through default pagination parameters to the search with page_size=30" do
expect(PactBroker::Index::Service).to receive(:find_index_items).with(hash_including(page_number: 1, page_size: 30))
expect(PactBroker::Index::Service).to receive(:find_index_items).with(hash_including(page_number: 1, page_size: 30)).and_call_original
get "/", { tags: "true" }
end
end

context "when not tags=true" do
it "passes through default pagination parameters to the search with page_size=100" do
expect(PactBroker::Index::Service).to receive(:find_index_items).with(hash_including(page_number: 1, page_size: 100))
expect(PactBroker::Index::Service).to receive(:find_index_items).with(hash_including(page_number: 1, page_size: 100)).and_call_original
get "/"
end
end
Expand Down

0 comments on commit 5d9d700

Please sign in to comment.