Skip to content

Commit

Permalink
Revert "Fix incompatible desirialization of environment variables"
Browse files Browse the repository at this point in the history
This reverts commit addba80.
  • Loading branch information
zrob committed Mar 20, 2015
1 parent 7a865a2 commit af5be06
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 25 deletions.
12 changes: 8 additions & 4 deletions app/models/runtime/app.rb
Expand Up @@ -303,11 +303,15 @@ def debug
self.metadata && self.metadata['debug']
end

def environment_json_with_serialization=(env)
self.environment_json_without_serialization = MultiJson.dump(env)
end
alias_method_chain :environment_json=, 'serialization'

def environment_json_with_serialization
environment_variables = environment_json_without_serialization
return environment_variables if environment_variables.is_a?(Hash)
return if environment_variables.blank?
MultiJson.load environment_variables
string = environment_json_without_serialization
return if string.blank?
MultiJson.load string
end
alias_method_chain :environment_json, 'serialization'

Expand Down
12 changes: 8 additions & 4 deletions app/models/runtime/environment_variable_group.rb
Expand Up @@ -13,11 +13,15 @@ def self.staging
find_by_name(:staging)
end

def environment_json_with_serialization=(env)
self.environment_json_without_serialization = MultiJson.dump(env)
end
alias_method_chain :environment_json=, 'serialization'

def environment_json_with_serialization
environment_variables = environment_json_without_serialization
return environment_variables if environment_variables.is_a?(Hash)
return {} if environment_variables.blank?
MultiJson.load environment_variables
string = environment_json_without_serialization
return {} if string.blank?
MultiJson.load string
end
alias_method_chain :environment_json, 'serialization'

Expand Down
2 changes: 1 addition & 1 deletion spec/support/shared_examples/models/encrypted_attribute.rb
Expand Up @@ -24,7 +24,7 @@ def last_row

it 'is encrypted before being written to the database' do
saved_attribute = last_row[storage_column]
expect(saved_attribute).not_to include YAML.dump(value_to_encrypt)
expect(saved_attribute).not_to include value_to_encrypt
end

it 'is decrypted when it is read from the database' do
Expand Down
4 changes: 2 additions & 2 deletions spec/unit/lib/cloud_controller/diego/environment_spec.rb
Expand Up @@ -5,8 +5,8 @@ module VCAP::CloudController::Diego
let(:app) do
app = VCAP::CloudController::AppFactory.make
app.environment_json = {
'APP_KEY1' => 'APP_VAL1',
'APP_KEY2' => 'APP_VAL2',
APP_KEY1: 'APP_VAL1',
APP_KEY2: 'APP_VAL2',
}
app
end
Expand Down
10 changes: 2 additions & 8 deletions spec/unit/models/runtime/app_spec.rb
Expand Up @@ -32,8 +32,8 @@ def expect_no_validator(validator_class)
end

it_behaves_like 'a model with an encrypted attribute' do
let(:value_to_encrypt) { { 'foo' => 'bar' } }
let(:encrypted_attr) { :environment_json }
let(:value_to_encrypt) { '{"foo":"bar"}' }
let(:encrypted_attr) { :environment_json_without_serialization }
let(:storage_column) { :encrypted_environment_json }
let(:attr_salt) { :salt }
end
Expand Down Expand Up @@ -770,12 +770,6 @@ def self.it_always_sets_stack
expect(app.environment_json).to eq('jesse' => 'awesome')
end

it 'is resilient' do
app = AppFactory.make
app.environment_json = { 'a' => 'b' }
expect(app.environment_json).to eq('a' => 'b')
end

def self.it_does_not_mark_for_re_staging
it 'does not mark an app for restage' do
app = AppFactory.make(
Expand Down
Expand Up @@ -13,7 +13,7 @@

it 'does allow an array' do
app.environment_json = []
expect(app.environment_json).to eq(nil)
expect(validator).to validate_with_error(app, :environment_json, :invalid_environment)
end

it 'allows multiple variables' do
Expand Down
5 changes: 5 additions & 0 deletions spec/unit/models/runtime/environment_variable_group_spec.rb
Expand Up @@ -6,6 +6,11 @@ module VCAP::CloudController

it { is_expected.to have_timestamp_columns }

it_behaves_like 'a model with an encrypted attribute' do
let(:encrypted_attr) { :environment_json }
let(:attr_salt) { :salt }
end

describe 'Serialization' do
it { is_expected.to export_attributes :name, :environment_json }
it { is_expected.to import_attributes :environment_json }
Expand Down
10 changes: 5 additions & 5 deletions spec/unit/models/v3/mappers/process_mapper_spec.rb
Expand Up @@ -44,7 +44,7 @@ module VCAP::CloudController
'buildpack' => 'buildpack',
'health_check_timeout' => 3,
'docker_image' => 'docker_image',
'environment_json' => { 'env' => 'json' },
'environment_json' => 'env_json',
'type' => 'worker'
}
end
Expand All @@ -67,7 +67,7 @@ module VCAP::CloudController
expect(model.buildpack.url).to eq('buildpack')
expect(model.health_check_timeout).to eq(3)
expect(model.docker_image).to eq('docker_image:latest')
expect(model.environment_json).to eq({ 'env' => 'json' })
expect(model.environment_json).to eq('env_json')
expect(model.type).to eq('worker')
end

Expand All @@ -89,7 +89,7 @@ module VCAP::CloudController
expect(model.buildpack.url).to eq('buildpack')
expect(model.health_check_timeout).to eq(3)
expect(model.docker_image).to eq('docker_image:latest')
expect(model.environment_json).to eq({ 'env' => 'json' })
expect(model.environment_json).to eq('env_json')
expect(model.type).to eq('worker')
end
end
Expand Down Expand Up @@ -128,7 +128,7 @@ module VCAP::CloudController
'buildpack' => 'buildpack',
'health_check_timeout' => 3,
'docker_image' => 'docker_image',
'environment_json' => { 'env' => 'json' },
'environment_json' => 'env_json',
'type' => 'worker'
}
end
Expand All @@ -150,7 +150,7 @@ module VCAP::CloudController
expect(model.buildpack.url).to eq('buildpack')
expect(model.health_check_timeout).to eq(3)
expect(model.docker_image).to eq('docker_image:latest')
expect(model.environment_json).to eq({ 'env' => 'json' })
expect(model.environment_json).to eq('env_json')
expect(model.type).to eq('worker')
end

Expand Down

0 comments on commit af5be06

Please sign in to comment.