Skip to content

Commit

Permalink
Merge pull request cloudfoundry#1620 from cloudfoundry/space_summary_…
Browse files Browse the repository at this point in the history
…wrong_guid_1523

V2 endpoints should return app guid instead of process guid in various places
  • Loading branch information
sethboyles authored May 21, 2020
2 parents 0278a44 + 0ae0b8a commit 0a4ff67
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 4 deletions.
4 changes: 4 additions & 0 deletions app/controllers/runtime/apps_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,10 @@ def upload_droplet(guid)
[HTTP::CREATED, JobPresenter.new(enqueued_job).to_json]
end

def url_for_guid(_, process)
super(process.app_guid, process)
end

def read(guid)
process = find_guid(guid)
raise CloudController::Errors::ApiError.new_from_details('AppNotFound', guid) unless process.web?
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/runtime/space_summaries_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def app_summary(space)
instances = instances_reporters.number_of_starting_and_running_instances_for_processes(space.apps)
space.apps.reject { |process| process.app.nil? }.collect do |process|
{
guid: process.guid,
guid: process.app_guid,
urls: process.routes.map(&:uri),
routes: process.routes.map(&:as_summary_json),
service_count: process.service_bindings_dataset.count,
Expand Down
121 changes: 121 additions & 0 deletions spec/unit/controllers/runtime/apps_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,127 @@ def update_app
end
end

describe 'read app' do
let(:process) { ProcessModelFactory.make(instances: 1) }
let(:app_model) { process.app }
let(:developer) { make_developer_for_space(process.space) }
let(:app_guid) { process.app_guid }

before do
set_current_user(developer)
allow_any_instance_of(V2::AppStage).to receive(:stage).and_return(nil)
end

it 'returns the app in question' do
get "/v2/apps/#{app_guid}"
expect(decoded_response['metadata']).to include({
'guid' => app_guid.to_s,
'url' => "/v2/apps/#{app_guid}",
'created_at' => anything,
'updated_at' => anything
})
expect(decoded_response['entity']).to include({
'name' => 'name-1',
'production' => false,
'space_guid' => process.space.guid.to_s,
'stack_guid' => process.stack.guid.to_s,
'buildpack' => nil,
'detected_buildpack' => nil,
'detected_buildpack_guid' => nil,
'environment_json' => nil,
'memory' => 1024,
'instances' => 1,
'disk_quota' => 1024,
'state' => 'STOPPED',
'version' => anything,
'command' => nil,
'console' => false,
'debug' => nil,
'staging_task_id' => anything,
'package_state' => 'STAGED',
'health_check_type' => 'port',
'health_check_timeout' => nil,
'health_check_http_endpoint' => nil,
'staging_failed_reason' => nil,
'staging_failed_description' => nil,
'diego' => true,
'docker_image' => nil,
'docker_credentials' => {
'username' => nil,
'password' => nil
},
'package_updated_at' => anything,
'detected_start_command' => '$HOME/boot.sh',
'enable_ssh' => true,
'ports' => [8080],
'space_url' => "/v2/spaces/#{process.space.guid}",
'stack_url' => "/v2/stacks/#{process.stack.guid}",
'routes_url' => "/v2/apps/#{app_guid}/routes",
'events_url' => "/v2/apps/#{app_guid}/events",
'service_bindings_url' => "/v2/apps/#{app_guid}/service_bindings",
'route_mappings_url' => "/v2/apps/#{app_guid}/route_mappings"
})
end

context 'when the app has rolled to a new web process' do
before do
process.destroy
ProcessModelFactory.make(instances: 1, app: app_model)
end

it 'returns the app with the appropriate app guid' do
get "/v2/apps/#{app_guid}"
expect(decoded_response['metadata']).to include({
'guid' => app_guid.to_s,
'url' => "/v2/apps/#{app_guid}",
'created_at' => anything,
'updated_at' => anything
})
expect(decoded_response['entity']).to include({
'name' => 'name-1',
'production' => false,
'space_guid' => process.space.guid.to_s,
'stack_guid' => process.stack.guid.to_s,
'buildpack' => nil,
'detected_buildpack' => nil,
'detected_buildpack_guid' => nil,
'environment_json' => nil,
'memory' => 1024,
'instances' => 1,
'disk_quota' => 1024,
'state' => 'STOPPED',
'version' => anything,
'command' => nil,
'console' => false,
'debug' => nil,
'staging_task_id' => anything,
'package_state' => 'STAGED',
'health_check_type' => 'port',
'health_check_timeout' => nil,
'health_check_http_endpoint' => nil,
'staging_failed_reason' => nil,
'staging_failed_description' => nil,
'diego' => true,
'docker_image' => nil,
'docker_credentials' => {
'username' => nil,
'password' => nil
},
'package_updated_at' => anything,
'detected_start_command' => '$HOME/boot.sh',
'enable_ssh' => true,
'ports' => [8080],
'space_url' => "/v2/spaces/#{process.space.guid}",
'stack_url' => "/v2/stacks/#{process.stack.guid}",
'routes_url' => "/v2/apps/#{app_guid}/routes",
'events_url' => "/v2/apps/#{app_guid}/events",
'service_bindings_url' => "/v2/apps/#{app_guid}/service_bindings",
'route_mappings_url' => "/v2/apps/#{app_guid}/route_mappings"
})
end
end
end

describe 'update app' do
let(:update_hash) { {} }

Expand Down
21 changes: 18 additions & 3 deletions spec/unit/controllers/runtime/space_summaries_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ module VCAP::CloudController
RSpec.describe SpaceSummariesController do
let(:space) { Space.make }
let(:process) { ProcessModelFactory.make(space: space) }
let(:app_model) { process.app }
let!(:first_route) { Route.make(space: space) }
let!(:second_route) { Route.make(space: space) }
let(:first_service) { ManagedServiceInstance.make(space: space) }
let(:second_service) { ManagedServiceInstance.make(space: space) }

let(:instances_reporters) { double(:instances_reporters) }
let(:running_instances) { { process.guid => 5 } }
let(:running_instances) { { app_model.guid => 5 } }

before do
ServiceBinding.make(app: process.app, service_instance: first_service)
Expand All @@ -37,7 +38,7 @@ module VCAP::CloudController
it 'returns the space apps' do
get "/v2/spaces/#{space.guid}/summary"
expected_app_hash = {
guid: process.guid,
guid: app_model.guid,
urls: [first_route.uri, second_route.uri],
routes: [
first_route.as_summary_json,
Expand Down Expand Up @@ -157,6 +158,7 @@ module VCAP::CloudController

context 'when an app is deleted concurrently' do
let(:deleted_process) { ProcessModelFactory.make(space: space) }
let!(:deleted_app_guid) { deleted_process.app.guid }
before do
deleted_process.app = nil
allow_any_instance_of(Space).to receive(:apps).and_return([process, deleted_process])
Expand All @@ -166,7 +168,20 @@ module VCAP::CloudController
get "/v2/spaces/#{space.guid}/summary"
expect(last_response.status).to eq(200)
expect(space.apps).to match([process, deleted_process])
expect(last_response.body).not_to include(deleted_process.guid)
expect(last_response.body).not_to include(deleted_app_guid)
end
end

context 'when the app has rolled to a new web process' do
before do
process.destroy
ProcessModel.make(app: app_model, type: ProcessTypes::WEB)
end

it 'returns the space apps with the appropriate app guid' do
get "/v2/spaces/#{space.guid}/summary"

expect(decoded_response['apps'][0]).to include({ 'guid' => app_model.guid })
end
end
end
Expand Down

0 comments on commit 0a4ff67

Please sign in to comment.