diff --git a/app/actions/space_diff_manifest.rb b/app/actions/space_diff_manifest.rb index 63992b66a67..1ab45b5bf2f 100644 --- a/app/actions/space_diff_manifest.rb +++ b/app/actions/space_diff_manifest.rb @@ -14,7 +14,9 @@ class << self # rubocop:todo Metrics/CyclomaticComplexity def generate_diff(app_manifests, space) json_diff = [] - recognized_top_level_keys = AppManifestMessage.allowed_keys.map(&:to_s) + + recognized_top_level_keys = AppManifestMessage.allowed_keys.map(&:to_s).map(&:dasherize) + app_manifests = normalize_units(app_manifests) app_manifests.each_with_index do |manifest_app_hash, index| manifest_app_hash = filter_manifest_app_hash(manifest_app_hash) @@ -99,7 +101,7 @@ def filter_manifest_app_hash(manifest_app_hash) 'type', 'command', 'disk_quota', - 'log_rate_limit_per_second', + 'log-rate-limit-per-second', 'health-check-http-endpoint', 'health-check-invocation-timeout', 'health-check-type', @@ -161,7 +163,7 @@ def normalize_units(manifest_app_hash) end end - byte_measurement_key_words = ['log_rate_limit_per_second'] + byte_measurement_key_words = ['log-rate-limit-per-second'] manifest_app_hash.each_with_index do |process_hash, index| byte_measurement_key_words.each do |key| value = process_hash[key] diff --git a/app/presenters/v3/app_manifest_presenters/process_properties_presenter.rb b/app/presenters/v3/app_manifest_presenters/process_properties_presenter.rb index 82180777924..904e5a8a9db 100644 --- a/app/presenters/v3/app_manifest_presenters/process_properties_presenter.rb +++ b/app/presenters/v3/app_manifest_presenters/process_properties_presenter.rb @@ -14,7 +14,7 @@ def process_hash(process) 'instances' => process.instances, 'memory' => add_units(process.memory), 'disk_quota' => add_units(process.disk_quota), - 'log_rate_limit_per_second' => add_units_log_rate_limit(process.log_rate_limit), + 'log-rate-limit-per-second' => add_units_log_rate_limit(process.log_rate_limit), 'command' => process.command, 'health-check-type' => process.health_check_type, 'health-check-http-endpoint' => process.health_check_http_endpoint, diff --git a/spec/request/app_manifests_spec.rb b/spec/request/app_manifests_spec.rb index 62a27bddaf7..14ad274fcb0 100644 --- a/spec/request/app_manifests_spec.rb +++ b/spec/request/app_manifests_spec.rb @@ -108,7 +108,7 @@ 'instances' => process.instances, 'memory' => "#{process.memory}M", 'disk_quota' => "#{process.disk_quota}M", - 'log_rate_limit_per_second' => '1M', + 'log-rate-limit-per-second' => '1M', 'health-check-type' => process.health_check_type, }, { @@ -116,7 +116,7 @@ 'instances' => worker_process.instances, 'memory' => "#{worker_process.memory}M", 'disk_quota' => "#{worker_process.disk_quota}M", - 'log_rate_limit_per_second' => '1M', + 'log-rate-limit-per-second' => '1M', 'command' => worker_process.command, 'health-check-type' => worker_process.health_check_type, 'health-check-http-endpoint' => worker_process.health_check_http_endpoint, @@ -207,7 +207,7 @@ 'instances' => process.instances, 'memory' => "#{process.memory}M", 'disk_quota' => "#{process.disk_quota}M", - 'log_rate_limit_per_second' => -1, + 'log-rate-limit-per-second' => -1, 'health-check-type' => process.health_check_type, }, { @@ -215,7 +215,7 @@ 'instances' => worker_process.instances, 'memory' => "#{worker_process.memory}M", 'disk_quota' => "#{worker_process.disk_quota}M", - 'log_rate_limit_per_second' => '1M', + 'log-rate-limit-per-second' => '1M', 'command' => worker_process.command, 'health-check-type' => worker_process.health_check_type, 'health-check-http-endpoint' => worker_process.health_check_http_endpoint, diff --git a/spec/request/space_manifests_spec.rb b/spec/request/space_manifests_spec.rb index 529eac982b1..7fe1263b092 100644 --- a/spec/request/space_manifests_spec.rb +++ b/spec/request/space_manifests_spec.rb @@ -27,7 +27,7 @@ 'instances' => 4, 'memory' => '2048MB', 'disk_quota' => '1.5GB', - 'log_rate_limit_per_second' => '1MB', + 'log-rate-limit-per-second' => '1MB', 'buildpack' => buildpack.name, 'stack' => buildpack.stack, 'command' => 'new-command', @@ -74,7 +74,7 @@ 'instances' => 3, 'memory' => '2048MB', 'disk_quota' => '1.5GB', - 'log_rate_limit_per_second' => '100KB', + 'log-rate-limit-per-second' => '100KB', 'buildpack' => buildpack.name, 'stack' => buildpack.stack, 'command' => 'newer-command', @@ -247,7 +247,7 @@ 'instances' => 4, 'memory' => '2048MB', 'disk_quota' => '1.5GB', - 'log_rate_limit_per_second' => '1GB', + 'log-rate-limit-per-second' => '1GB', 'buildpack' => buildpack.name, 'stack' => buildpack.stack, 'command' => 'new-command', @@ -271,7 +271,7 @@ 'instances' => 4, 'memory' => '2048MB', 'disk_quota' => '1.5GB', - 'log_rate_limit_per_second' => '-1B', + 'log-rate-limit-per-second' => '-1B', 'buildpack' => buildpack.name, 'stack' => buildpack.stack, 'command' => 'new-command', @@ -467,7 +467,7 @@ 'version' => 1, 'applications' => [ { 'name' => app1_model.name, - 'log_rate_limit_per_second' => -1 + 'log-rate-limit-per-second' => -1 }, ] }.to_yaml @@ -521,7 +521,7 @@ 'instances' => 4, 'memory' => '2048MB', 'disk_quota' => '1.5GB', - 'log_rate_limit_per_second' => '300B', + 'log-rate-limit-per-second' => '300B', 'buildpack' => buildpack.name, 'stack' => buildpack.stack, 'command' => 'new-command', diff --git a/spec/unit/actions/space_diff_manifest_spec.rb b/spec/unit/actions/space_diff_manifest_spec.rb index a02b26a5cf3..1c8b4a849a8 100644 --- a/spec/unit/actions/space_diff_manifest_spec.rb +++ b/spec/unit/actions/space_diff_manifest_spec.rb @@ -21,7 +21,7 @@ module VCAP::CloudController 'instances' => process1.instances, 'memory' => '1024M', 'disk_quota' => '1024M', - 'log_rate_limit_per_second' => '1M', + 'log-rate-limit-per-second' => '1M', 'health-check-type' => process1.health_check_type }, { @@ -29,7 +29,7 @@ module VCAP::CloudController 'instances' => process2.instances, 'memory' => '1024M', 'disk_quota' => '1024M', - 'log_rate_limit_per_second' => '1M', + 'log-rate-limit-per-second' => '1M', 'health-check-type' => process2.health_check_type } ] @@ -71,13 +71,13 @@ module VCAP::CloudController context 'when there are changes in the manifest' do before do - default_manifest['applications'][0]['random_route'] = true + default_manifest['applications'][0]['random-route'] = true default_manifest['applications'][0]['stack'] = 'big brother' end it 'returns the correct diff' do expect(subject).to match_array([ - { 'op' => 'add', 'path' => '/applications/0/random_route', 'value' => true }, + { 'op' => 'add', 'path' => '/applications/0/random-route', 'value' => true }, { 'op' => 'replace', 'path' => '/applications/0/stack', 'was' => process1.stack.name, 'value' => 'big brother' }, ]) end @@ -266,7 +266,7 @@ module VCAP::CloudController before do default_manifest['applications'][0]['processes'][0]['memory'] = '1G' default_manifest['applications'][0]['processes'][0]['disk_quota'] = '1G' - default_manifest['applications'][0]['processes'][0]['log_rate_limit_per_second'] = '1024K' + default_manifest['applications'][0]['processes'][0]['log-rate-limit-per-second'] = '1024K' end it 'returns an empty diff' do expect(subject).to eq([]) @@ -277,13 +277,15 @@ module VCAP::CloudController before do default_manifest['applications'][0]['processes'][0]['memory'] = '2G' default_manifest['applications'][0]['processes'][0]['disk_quota'] = '4G' - default_manifest['applications'][0]['processes'][0]['log_rate_limit_per_second'] = '2G' + default_manifest['applications'][0]['processes'][0]['health-check-type'] = 'process' + default_manifest['applications'][0]['processes'][0]['log-rate-limit-per-second'] = '2G' end it 'returns the diff formatted' do expect(subject).to eq([ { 'op' => 'replace', 'path' => '/applications/0/processes/0/memory', 'value' => '2048M', 'was' => '1024M' }, { 'op' => 'replace', 'path' => '/applications/0/processes/0/disk_quota', 'value' => '4096M', 'was' => '1024M' }, - { 'op' => 'replace', 'path' => '/applications/0/processes/0/log_rate_limit_per_second', 'value' => '2G', 'was' => '1M' }, + { 'op' => 'replace', 'path' => '/applications/0/processes/0/log-rate-limit-per-second', 'value' => '2G', 'was' => '1M' }, + { 'op' => 'replace', 'path' => '/applications/0/processes/0/health-check-type', 'value' => 'process', 'was' => 'port' }, ]) end end @@ -322,29 +324,71 @@ module VCAP::CloudController end context 'when updating app-level configurations' do - before do - default_manifest['applications'][0]['memory'] = '1G' - default_manifest['applications'][0]['disk_quota'] = '1G' - default_manifest['applications'][0]['log_rate_limit_per_second'] = '1024K' + context 'when nothing has changed' do + before do + default_manifest['applications'][0]['log-rate-limit-per-second'] = '1024K' + end + + it 'returns an empty diff if the field is equivalent' do + expect(subject).to eq([]) + end end - it 'returns an empty diff if the field is equivalent' do - expect(subject).to eq([]) + context 'when trying to change memory and disk quota' do + before do + default_manifest['applications'][0]['memory'] = '99G' + default_manifest['applications'][0]['disk_quota'] = '99G' + end + + it 'returns an empty diff because for some reason we ignore these fields at the app level' do + expect(subject).to eq([]) + end + end + + context 'when things have changed' do + before do + default_manifest['applications'][0]['health-check-type'] = 'process' + default_manifest['applications'][0]['instances'] = 9 + default_manifest['applications'][0]['log-rate-limit-per-second'] = '1G' + end + + it 'displays in the diff' do + expect(subject).to eq([ + { + 'op' => 'replace', + 'path' => '/applications/0/health-check-type', + 'was' => 'port', + 'value' => 'process' + }, + { + 'op' => 'replace', + 'path' => '/applications/0/instances', + 'was' => 1, + 'value' => 9 + }, + { + 'op' => 'replace', + 'path' => '/applications/0/log-rate-limit-per-second', + 'was' => '1M', + 'value' => '1G' + } + ]) + end end end end - context 'when the log_rate_limit_per_second is unlimited' do + context 'when the log-rate-limit-per-second is unlimited' do before do - default_manifest['applications'][0]['log_rate_limit_per_second'] = -1 + default_manifest['applications'][0]['log-rate-limit-per-second'] = '-1B' end it 'displays -1 in the diff' do expect(subject).to include( { 'op' => 'replace', - 'path' => '/applications/0/log_rate_limit_per_second', - 'value' => -1, + 'path' => '/applications/0/log-rate-limit-per-second', + 'value' => '-1B', 'was' => '1M' } ) diff --git a/spec/unit/messages/app_manifest_message_spec.rb b/spec/unit/messages/app_manifest_message_spec.rb index d3c4f19b769..37cb3030eb3 100644 --- a/spec/unit/messages/app_manifest_message_spec.rb +++ b/spec/unit/messages/app_manifest_message_spec.rb @@ -104,8 +104,8 @@ module VCAP::CloudController end end - describe 'log_rate_limit_per_second' do - context 'when log_rate_limit_per_second unit is not part of expected set of values' do + describe 'log-rate-limit-per-second' do + context 'when log-rate-limit-per-second unit is not part of expected set of values' do let(:params_from_yaml) { { name: 'eugene', log_rate_limit_per_second: '200INVALID' } } it 'is not valid' do @@ -119,7 +119,7 @@ module VCAP::CloudController end end - context 'when log_rate_limit_per_second is less than -1 bytes' do + context 'when log-rate-limit-per-second is less than -1 bytes' do let(:params_from_yaml) { { name: 'eugene', log_rate_limit_per_second: '-1M' } } it 'is not valid' do @@ -131,7 +131,7 @@ module VCAP::CloudController end end - context 'when log_rate_limit_per_second is not numeric' do + context 'when log-rate-limit-per-second is not numeric' do let(:params_from_yaml) { { name: 'eugene', log_rate_limit_per_second: 'gerg herscheisers' } } it 'is not valid' do @@ -145,7 +145,7 @@ module VCAP::CloudController end end - context 'when log_rate_limit_per_second is an unlimited amount' do + context 'when log-rate-limit-per-second is an unlimited amount' do context 'specified as -1' do let(:params_from_yaml) { { name: 'eugene', log_rate_limit_per_second: -1 } } @@ -164,7 +164,7 @@ module VCAP::CloudController end end - context 'when log_rate_limit_per_second is in megabytes' do + context 'when log-rate-limit-per-second is in megabytes' do let(:params_from_yaml) { { name: 'eugene', log_rate_limit_per_second: '1MB' } } it 'is valid' do @@ -642,7 +642,7 @@ module VCAP::CloudController 'instances' => -30, 'memory' => 'potato', 'disk_quota' => '100', - 'log_rate_limit_per_second' => 'kumara', + 'log-rate-limit-per-second' => 'kumara', 'health_check_type' => 'sweet_potato', 'health_check_http_endpoint' => '/healthcheck_potato', 'health_check_invocation_timeout' => 'yucca', @@ -656,7 +656,7 @@ module VCAP::CloudController 'instances' => 'cassava', 'memory' => 'potato', 'disk_quota' => '100', - 'log_rate_limit_per_second' => '100', + 'log-rate-limit-per-second' => '100', 'health_check_type' => 'sweet_potato', 'health_check_http_endpoint' => '/healthcheck_potato', 'health_check_invocation_timeout' => 'yucca', @@ -714,10 +714,10 @@ module VCAP::CloudController end end - context 'log_rate_limit_per_second' do - context 'when log_rate_limit_per_second is an unlimited amount' do + context 'log-rate-limit-per-second' do + context 'when log-rate-limit-per-second is an unlimited amount' do context 'specified as -1' do - let(:params_from_yaml) { { name: 'eugene', processes: [{ 'type' => 'foo', 'log_rate_limit_per_second' => -1 }] } } + let(:params_from_yaml) { { name: 'eugene', processes: [{ 'type' => 'foo', 'log-rate-limit-per-second' => -1 }] } } it 'is valid' do message = AppManifestMessage.create_from_yml(params_from_yaml) @@ -725,7 +725,7 @@ module VCAP::CloudController end end context 'with a bytes suffix' do - let(:params_from_yaml) { { name: 'eugene', processes: [{ 'type' => 'foo', 'log_rate_limit_per_second' => '-1B' }] } } + let(:params_from_yaml) { { name: 'eugene', processes: [{ 'type' => 'foo', 'log-rate-limit-per-second' => '-1B' }] } } it 'is valid' do message = AppManifestMessage.create_from_yml(params_from_yaml) diff --git a/spec/unit/presenters/v3/app_manifest_presenter_spec.rb b/spec/unit/presenters/v3/app_manifest_presenter_spec.rb index 8d76975cc81..31df7131d89 100644 --- a/spec/unit/presenters/v3/app_manifest_presenter_spec.rb +++ b/spec/unit/presenters/v3/app_manifest_presenter_spec.rb @@ -120,7 +120,7 @@ module VCAP::CloudController::Presenters::V3 'type' => process1.type, 'instances' => process1.instances, 'memory' => "#{process1.memory}M", - 'log_rate_limit_per_second' => '1M', + 'log-rate-limit-per-second' => '1M', 'disk_quota' => "#{process1.disk_quota}M", 'command' => process1.command, 'health-check-type' => process1.health_check_type, @@ -130,7 +130,7 @@ module VCAP::CloudController::Presenters::V3 { 'type' => process2.type, 'instances' => process2.instances, - 'log_rate_limit_per_second' => '1M', + 'log-rate-limit-per-second' => '1M', 'memory' => "#{process2.memory}M", 'disk_quota' => "#{process2.disk_quota}M", 'health-check-type' => process2.health_check_type, @@ -169,7 +169,7 @@ module VCAP::CloudController::Presenters::V3 { 'type' => process1.type, 'instances' => process1.instances, - 'log_rate_limit_per_second' => '1M', + 'log-rate-limit-per-second' => '1M', 'memory' => "#{process1.memory}M", 'disk_quota' => "#{process1.disk_quota}M", 'health-check-type' => process1.health_check_type, @@ -177,7 +177,7 @@ module VCAP::CloudController::Presenters::V3 { 'type' => process2.type, 'instances' => process2.instances, - 'log_rate_limit_per_second' => '1M', + 'log-rate-limit-per-second' => '1M', 'memory' => "#{process2.memory}M", 'disk_quota' => "#{process2.disk_quota}M", 'health-check-type' => process2.health_check_type, diff --git a/spec/unit/presenters/v3/app_manifest_presenters/process_properties_presenter_spec.rb b/spec/unit/presenters/v3/app_manifest_presenters/process_properties_presenter_spec.rb index 50e19d29680..5945871e56a 100644 --- a/spec/unit/presenters/v3/app_manifest_presenters/process_properties_presenter_spec.rb +++ b/spec/unit/presenters/v3/app_manifest_presenters/process_properties_presenter_spec.rb @@ -43,7 +43,7 @@ module VCAP::CloudController::Presenters::V3::AppManifestPresenters 'instances' => 1, 'memory' => '1024M', 'disk_quota' => '1024M', - 'log_rate_limit_per_second' => '1M', + 'log-rate-limit-per-second' => '1M', 'health-check-type' => 'port', }) end