Skip to content

Commit

Permalink
Consolidate the external API of all service objects
Browse files Browse the repository at this point in the history
- Don't duplicate the methods signature in static facades

  Since the method signature, including its parameters are already
  present in the initializer. We can safely remove it from the static
  facade without hurting readability.

- Rename `.run` to `.call`.
- Rename `.for` with `.call`
- Rename `#run` to `#call`.
- Add static facades where they are missing.
  • Loading branch information
teoljungberg committed Jan 11, 2017
1 parent d27222f commit 32336a2
Show file tree
Hide file tree
Showing 43 changed files with 113 additions and 137 deletions.
2 changes: 1 addition & 1 deletion app/controllers/builds_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,6 @@ def payload
end

def recent_builds_by_repo
RecentBuildsByRepoQuery.run(user: current_user)
RecentBuildsByRepoQuery.call(user: current_user)
end
end
2 changes: 1 addition & 1 deletion app/controllers/repos_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def index
current_user.repos.clear
end

repos = ReposWithMembershipOrSubscriptionQuery.new(current_user).run
repos = ReposWithMembershipOrSubscriptionQuery.call(current_user)

render json: repos
end
Expand Down
5 changes: 2 additions & 3 deletions app/jobs/buildable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ def perform(payload_data)
payload = Payload.new(payload_data)

unless blacklisted?(payload)
UpdateRepoStatus.new(payload).run
build_runner = BuildRunner.new(payload)
build_runner.run
UpdateRepoStatus.call(payload)
BuildRunner.call(payload)
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/jobs/completed_file_review_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class CompletedFileReviewJob
# violations
# [{ line: 123, message: "WAT" }]
def self.perform(attributes)
CompleteFileReview.run(attributes)
CompleteFileReview.call(attributes)
rescue ActiveRecord::RecordNotFound
Resque.enqueue_in(30, self, attributes)
rescue Resque::TermException
Expand Down
2 changes: 1 addition & 1 deletion app/jobs/report_invalid_config_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class ReportInvalidConfigJob
# The following parameters are optional:
# - message
def self.perform(attributes)
ReportInvalidConfig.run(
ReportInvalidConfig.call(
pull_request_number: attributes.fetch("pull_request_number"),
commit_sha: attributes.fetch("commit_sha"),
linter_name: attributes.fetch("linter_name"),
Expand Down
6 changes: 3 additions & 3 deletions app/models/hound_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,15 @@ def default_config
end

def resolved_aliases_config
ResolveConfigAliases.run(normalized_config)
ResolveConfigAliases.call(normalized_config)
end

def normalized_config
NormalizeConfig.run(parsed_config)
NormalizeConfig.call(parsed_config)
end

def resolved_conflicts_config
ResolveConfigConflicts.run(resolved_aliases_config)
ResolveConfigConflicts.call(resolved_aliases_config)
end

def parsed_config
Expand Down
7 changes: 2 additions & 5 deletions app/models/jshint_config_builder.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
class JshintConfigBuilder
static_facade :call
pattr_initialize :hound_config

def self.for(hound_config)
new(hound_config).config
end

def config
def call
Config::Jshint.new(load_content)
end

Expand Down
2 changes: 1 addition & 1 deletion app/models/linter/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def owner
end

def config
@_config ||= BuildConfig.for(
@_config ||= BuildConfig.call(
hound_config: hound_config,
name: name,
owner: owner,
Expand Down
6 changes: 3 additions & 3 deletions app/models/linter/jshint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ def config
end

def local_config
@_config ||= JshintConfigBuilder.for(hound_config)
@_config ||= JshintConfigBuilder.call(hound_config)
end

def owner_config
@_owner_config ||= JshintConfigBuilder.for(owner_hound_config)
@_owner_config ||= JshintConfigBuilder.call(owner_hound_config)
end

def owner_hound_config
BuildOwnerHoundConfig.run(build.repo.owner)
BuildOwnerHoundConfig.call(build.repo.owner)
end

def jsignore
Expand Down
4 changes: 2 additions & 2 deletions app/models/owner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ def has_config_repo?
end

def config_content(linter_name)
BuildConfig.for(
hound_config: BuildOwnerHoundConfig.run(self),
BuildConfig.call(
hound_config: BuildOwnerHoundConfig.call(self),
name: linter_name,
owner: MissingOwner.new,
).content
Expand Down
6 changes: 2 additions & 4 deletions app/queries/recent_builds_by_repo_query.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
class RecentBuildsByRepoQuery
NUMBER_OF_BUILDS = 20

def self.run(*args)
new(*args).run
end
static_facade :call

def initialize(user:)
@user = user
end

def run
def call
Build.find_by_sql([<<-SQL, user_id: @user.id, limit: NUMBER_OF_BUILDS])
WITH user_builds AS (
SELECT
Expand Down
4 changes: 3 additions & 1 deletion app/queries/repos_with_membership_or_subscription_query.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
class ReposWithMembershipOrSubscriptionQuery
static_facade :call

def initialize(user)
@user = user
end

def run
def call
@user.subscribed_repos.includes(:subscription) |
@user.repos_by_activation_ability.includes(:subscription)
end
Expand Down
6 changes: 2 additions & 4 deletions app/services/build_config.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
class BuildConfig
def self.for(hound_config:, name:, owner:)
new(hound_config: hound_config, name: name, owner: owner).config
end
static_facade :call

def initialize(hound_config:, name:, owner:)
@hound_config = hound_config
@name = name
@owner = owner
end

def config
def call
config_class.new(hound_config, owner: owner)
end

Expand Down
6 changes: 2 additions & 4 deletions app/services/build_owner_hound_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@
class BuildOwnerHoundConfig
LATEST_SHA = "HEAD"

def self.run(*args)
new(*args).run
end
static_facade :call

def initialize(owner)
@owner = owner
end

def run
def call
if owner.has_config_repo? && config_repo_reachable?
commit = Commit.new(config_repo.name, LATEST_SHA, github)
HoundConfig.new(commit)
Expand Down
5 changes: 3 additions & 2 deletions app/services/build_runner.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
class BuildRunner
static_facade :call
pattr_initialize :payload

def run
def call
if repo && relevant_pull_request?
review_pull_request
end
Expand Down Expand Up @@ -88,7 +89,7 @@ def commit_status
end

def report_config_file_as_invalid(exception)
ReportInvalidConfig.run(
ReportInvalidConfig.call(
pull_request_number: payload.pull_request_number,
commit_sha: payload.head_sha,
linter_name: exception.linter_name,
Expand Down
6 changes: 2 additions & 4 deletions app/services/complete_build.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
class CompleteBuild
def self.run(pull_request:, build:, token:)
new(pull_request: pull_request, build: build, token: token).run
end
static_facade :call

def initialize(pull_request:, build:, token:)
@build = build
Expand All @@ -10,7 +8,7 @@ def initialize(pull_request:, build:, token:)
@commenter = Commenter.new(pull_request)
end

def run
def call
if build.completed?
commenter.comment_on_violations(priority_violations)
set_commit_status
Expand Down
8 changes: 3 additions & 5 deletions app/services/complete_file_review.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
class CompleteFileReview
def self.run(attributes)
new(attributes).run
end
static_facade :call

def initialize(attributes)
@attributes = attributes
end

def run
def call
create_violations!

CompleteBuild.run(
CompleteBuild.call(
pull_request: pull_request,
build: build,
token: build.user_token,
Expand Down
8 changes: 3 additions & 5 deletions app/services/normalize_config.rb
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
class NormalizeConfig
def self.run(config)
new(config).run
end
static_facade :call

def initialize(config)
@config = config
end

def run
def call
@config.reduce({}) do |normalized_config, (key, value)|
normalized_key = normalize_key(key)
if value.is_a? Hash
normalized_config[normalized_key] = NormalizeConfig.run(value)
normalized_config[normalized_key] = NormalizeConfig.call(value)
else
normalized_config[normalized_key] = value
end
Expand Down
4 changes: 1 addition & 3 deletions app/services/rebuild_pull_request.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
class RebuildPullRequest
def self.call(*args)
new(*args).call
end
static_facade :call

def initialize(repo:, pull_request_number:)
@repo = repo
Expand Down
6 changes: 2 additions & 4 deletions app/services/report_invalid_config.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
class ReportInvalidConfig
def self.run(**args)
new(**args).run
end
static_facade :call

def initialize(pull_request_number:, commit_sha:, linter_name:, message: "")
@pull_request_number = pull_request_number
Expand All @@ -10,7 +8,7 @@ def initialize(pull_request_number:, commit_sha:, linter_name:, message: "")
@message = message
end

def run
def call
commit_status.set_config_error(message)
end

Expand Down
6 changes: 2 additions & 4 deletions app/services/resolve_config_aliases.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,13 @@ class ResolveConfigAliases
"coffeescript" => "coffee_script",
}.freeze

def self.run(config)
new(config).run
end
static_facade :call

def initialize(config)
@config = config
end

def run
def call
@config.reduce({}) do |resolved_config, (key, value)|
if ALIASES.keys.include? key
resolved_config[ALIASES[key]] = value
Expand Down
6 changes: 2 additions & 4 deletions app/services/resolve_config_conflicts.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
class ResolveConfigConflicts
CONFLICTS = { "eslint" => "jshint" }.freeze

def self.run(config)
new(config).run
end
static_facade :call

def initialize(config)
@config = config
end

def run
def call
@config.reduce({}) do |resolved_config, (linter, options)|
if CONFLICTS.has_key?(linter) && options["enabled"] == true
resolved_config[CONFLICTS[linter]] = { "enabled" => false }
Expand Down
3 changes: 2 additions & 1 deletion app/services/update_repo_status.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
class UpdateRepoStatus
static_facade :call
pattr_initialize :payload

def run
def call
if repo
repo.update(repo_attributes)
end
Expand Down
2 changes: 1 addition & 1 deletion spec/features/job_failures_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
Resque::Failure.backend = Resque::Failure::Redis
cleanup_test_failures

example.run
example.call

cleanup_test_failures
Resque::Failure.backend = previous_backend
Expand Down
12 changes: 4 additions & 8 deletions spec/jobs/buildable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,23 @@ class BuildableTestJob < ActiveJob::Base

describe "#perform" do
it 'runs build runner' do
build_runner = double(:build_runner, run: nil)
payload = stub_payload(payload_data)
allow(BuildRunner).to receive(:new).and_return(build_runner)
allow(BuildRunner).to receive(:call)

BuildableTestJob.perform_now(payload_data)

expect(Payload).to have_received(:new).with(payload_data)
expect(BuildRunner).to have_received(:new).with(payload)
expect(build_runner).to have_received(:run)
expect(BuildRunner).to have_received(:call).with(payload)
end

it "runs repo updater" do
repo_updater = double("RepoUpdater", run: nil)
payload = stub_payload(payload_data)
allow(UpdateRepoStatus).to receive(:new).and_return(repo_updater)
allow(UpdateRepoStatus).to receive(:call)

BuildableTestJob.perform_now(payload_data)

expect(Payload).to have_received(:new).with(payload_data)
expect(UpdateRepoStatus).to have_received(:new).with(payload)
expect(repo_updater).to have_received(:run)
expect(UpdateRepoStatus).to have_received(:call).with(payload)
end

context "when the pull request has been blacklisted" do
Expand Down
Loading

0 comments on commit 32336a2

Please sign in to comment.