Skip to content

Commit

Permalink
Refactor ApplyManifest to use separate strategies for command
Browse files Browse the repository at this point in the history
[#154739913]

Signed-off-by: An Yu <anyu@pivotal.io>
  • Loading branch information
Lisa Cho authored and anyu committed Mar 26, 2018
1 parent f40fe86 commit 5f588e8
Show file tree
Hide file tree
Showing 11 changed files with 99 additions and 219 deletions.
3 changes: 2 additions & 1 deletion app/actions/app_apply_manifest.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'actions/process_scale'
require 'cloud_controller/strategies/manifest_strategy'

module VCAP::CloudController
class AppApplyManifest
Expand All @@ -14,7 +15,7 @@ def apply(app_guid, message)
lifecycle = AppLifecycleProvider.provide_for_update(app_update_message, app)
AppUpdate.new(user_audit_info).update(app, app_update_message, lifecycle)

ProcessUpdate.new(user_audit_info).update(app.web_process, message.manifest_process_update_message)
ProcessUpdate.new(user_audit_info).update(app.web_process, message.manifest_process_update_message, ManifestStrategy)

AppPatchEnvironmentVariables.new(user_audit_info).patch(app, message.manifest_env_update_message)

Expand Down
2 changes: 1 addition & 1 deletion app/actions/current_process_types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def evaluate_processes(app, process_types)
def add_or_update_process(app, type, command)
existing_process = app.processes_dataset.where(type: type).first
if existing_process
ProcessUpdate.new(@user_audit_info).update(existing_process, ProcessUpdateMessage.new({ command: command }))
ProcessUpdate.new(@user_audit_info).update(existing_process, ProcessUpdateMessage.new({ command: command }), NonManifestStrategy)
else
ProcessCreate.new(@user_audit_info).create(app, { type: type, command: command })
end
Expand Down
35 changes: 0 additions & 35 deletions app/actions/process_manifest_update.rb

This file was deleted.

5 changes: 3 additions & 2 deletions app/actions/process_update.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ def initialize(user_audit_info)
@user_audit_info = user_audit_info
end

def update(process, message)
def update(process, message, strategy_class)
strategy = strategy_class.new(message, process)
process.db.transaction do
process.lock!

process.command = message.command if message.requested?(:command)
process.command = strategy.updated_command if message.requested?(:command)
process.health_check_type = message.health_check_type if message.requested?(:health_check_type)
process.health_check_timeout = message.health_check_timeout if message.requested?(:health_check_timeout)
if message.requested?(:health_check_type) && message.health_check_type != 'http'
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/v3/processes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
require 'messages/process_update_message'
require 'messages/processes_list_message'
require 'controllers/v3/mixins/app_sub_resource'
require 'cloud_controller/strategies/non_manifest_strategy'

class ProcessesController < ApplicationController
include AppSubResource
Expand Down Expand Up @@ -45,7 +46,7 @@ def update
message = ProcessUpdateMessage.create_from_http_request(unmunged_body)
unprocessable!(message.errors.full_messages) unless message.valid?

ProcessUpdate.new(user_audit_info).update(@process, message)
ProcessUpdate.new(user_audit_info).update(@process, message, NonManifestStrategy)

render status: :ok, json: Presenters::V3::ProcessPresenter.new(@process)
rescue ProcessUpdate::InvalidProcess => e
Expand Down
1 change: 0 additions & 1 deletion app/models/helpers/process_types.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
module VCAP::CloudController
class ProcessTypes
WEB = 'web'.freeze
NULL_START_COMMANDS = ['default', 'null'].freeze
end
end
18 changes: 18 additions & 0 deletions lib/cloud_controller/strategies/manifest_strategy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
module VCAP::CloudController
class ManifestStrategy
NULL_START_COMMANDS = ['default', 'null'].freeze

def initialize(message, process)
@process = process
@message = message
end

def updated_command
if NULL_START_COMMANDS.include?(@message.command)
@process.detected_start_command
elsif @message.command
@message.command
end
end
end
end
12 changes: 12 additions & 0 deletions lib/cloud_controller/strategies/non_manifest_strategy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module VCAP::CloudController
class NonManifestStrategy
def initialize(message, process)
@process = process
@message = message
end

def updated_command
@message.command
end
end
end
2 changes: 1 addition & 1 deletion spec/unit/actions/app_apply_manifest_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ module VCAP::CloudController
app_apply_manifest.apply(app.guid, message)
expect(ProcessUpdate).to have_received(:new).with(user_audit_info)
expect(process_update).to have_received(:update).
with(app.web_process, manifest_process_update_message)
with(app.web_process, manifest_process_update_message, ManifestStrategy)
end
end

Expand Down
169 changes: 0 additions & 169 deletions spec/unit/actions/process_manifest_update_spec.rb

This file was deleted.

0 comments on commit 5f588e8

Please sign in to comment.