Revert "Revert "Add transactional tests. Remove #reset_database. Add …

…random order.""

This reverts commit 85eef29.
continuing to work on transactional tests.

.rspec

@@ -1,4 +1,5 @@
 --backtrace
 --format progress
 --colour
---profile
+--profile
+--order rand

app/controllers/runtime/apps_controller.rb

@@ -58,7 +58,7 @@ def delete(guid)
         raise VCAP::Errors::AssociationNotEmpty.new("service_bindings", app.class.table_name)
       end
 
-      app.db.transaction do
+      app.db.transaction(savepoint: true) do
         app.soft_delete
         Event.record_app_delete(app, SecurityContext.current_user)
       end
@@ -68,7 +68,7 @@ def delete(guid)
 
     def update(guid)
       app = find_for_update(guid)
-      model.db.transaction do
+      model.db.transaction(savepoint: true) do
         app.lock!
         app.update_from_hash(request_attrs)
         Event.record_app_update(app, SecurityContext.current_user) if app.previous_changes
@@ -91,7 +91,7 @@ def create
       raise InvalidRequest unless request_attrs
 
       obj = nil
-      model.db.transaction do
+      model.db.transaction(savepoint: true) do
         obj = model.create_from_hash(request_attrs)
         validate_access(:create, obj, user, roles)
         Event.record_app_create(obj, SecurityContext.current_user)

app/controllers/runtime/buildpack_bits_controller.rb

@@ -20,7 +20,7 @@ def upload(guid)
       buildpack_blobstore.cp_to_blobstore(uploaded_file, sha1)
 
       old_buildpack_key = buildpack.key
-      model.db.transaction do
+      model.db.transaction(savepoint: true) do
         buildpack.lock!
         buildpack.update_from_hash(key: sha1)
       end

app/controllers/runtime/buildpacks_controller.rb

@@ -30,7 +30,7 @@ def update(guid)
 
       attrs = @request_attrs.dup
       target_position = attrs.delete('position')
-      model.db.transaction do
+      model.db.transaction(savepoint: true) do
         obj.lock!
         obj.update_from_hash(attrs)
         obj.shift_to_position(target_position)

app/models/runtime/app.rb

@@ -373,7 +373,7 @@ def cleanup_associations
     def soft_delete
       raise AlreadyDeletedError, "App: #{self} was already soft deleted on: #{deleted_at}" if deleted_at
 
-      model.db.transaction do
+      model.db.transaction(savepoint: true) do
         lock!
         cleanup_associations
         self.deleted_at = Time.now

app/models/runtime/buildpack.rb

@@ -27,7 +27,7 @@ def self.create(values = {}, &block)
       last = Buildpack.at_last_position
 
       if last
-        db.transaction do
+        db.transaction(savepoint: true) do
           last.lock!
           last_position = last.position
 
@@ -48,7 +48,7 @@ def self.create(values = {}, &block)
     def shift_to_position(target_position)
       return if target_position == position
 
-      db.transaction do
+      db.transaction(savepoint: true) do
         last = Buildpack.at_last_position
         if last
           last.lock!

app/models/runtime/domain.rb

@@ -136,7 +136,7 @@ def self.find_or_create_shared_domain(name)
       logger = Steno.logger("cc.db.domain")
       d = nil
 
-      Domain.db.transaction do
+      Domain.db.transaction(savepoint: true) do
         d = Domain[:name => name]
         if d
           logger.info "reusing default serving domain: #{name}"

app/models/services/service_broker_registration.rb

@@ -10,7 +10,7 @@ def save(options = {})
       ensure_no_raise_on_failure!(options)
 
       if broker.valid?
-        broker.db.transaction do
+        broker.db.transaction(savepoint: true) do
           broker.save
           broker.load_catalog
         end

lib/cloud_controller/legacy_api/legacy_service_gateway.rb

@@ -27,7 +27,7 @@ def create_offering
       validate_access(label, provider)
 
       set_v2_security_context
-      Sequel::Model.db.transaction do
+      Sequel::Model.db.transaction(savepoint: true) do
         service = Service.update_or_create(
           :label => label, :provider => provider
         ) do |svc|

lib/cloud_controller/rest_controller/model_controller.rb

@@ -19,7 +19,7 @@ def create
       before_create
 
       obj = nil
-      model.db.transaction do
+      model.db.transaction(savepoint: true) do
         obj = model.create_from_hash(request_attrs)
         validate_access(:create, obj, user, roles)
       end
@@ -49,7 +49,7 @@ def read(guid)
     def update(guid)
       obj = find_for_update(guid)
 
-      model.db.transaction do
+      model.db.transaction(savepoint: true) do
         obj.lock!
         obj.update_from_hash(request_attrs)
       end

lib/cloud_controller/seeds.rb

@@ -16,7 +16,7 @@ def create_seed_quota_definitions(config)
         end
       end
 
-      def create_seed_stacks(config)
+      def create_seed_stacks(_)
         Stack.populate
       end
 

spec/access/domain_access_spec.rb

@@ -89,11 +89,6 @@ module VCAP::CloudController
     context 'when the domain is not owned by any organization (global domain)' do
       before { object.update(:owning_organization => nil) }
 
-      after do
-        # Global domains cause test pollution.
-        object.destroy
-      end
-
       it_behaves_like :admin_full_access
       it_behaves_like :read_only
 

spec/api/apps_api_spec.rb

@@ -5,12 +5,7 @@
   let(:admin_auth_header) { headers_for(admin_user, :admin_scope => true)["HTTP_AUTHORIZATION"] }
   authenticated_request
 
-  before do
-    reset_database
-    3.times do
-      VCAP::CloudController::AppFactory.make
-    end
-  end
+  before { 3.times { VCAP::CloudController::AppFactory.make } }
 
   let(:admin_buildpack) { VCAP::CloudController::Buildpack.make }
 

spec/api/buildpacks_api_spec.rb

@@ -5,16 +5,6 @@
   let(:admin_auth_header) { headers_for(admin_user, :admin_scope => true)["HTTP_AUTHORIZATION"] }
   authenticated_request
 
-  before(:all) do
-    reset_database
-  end
-
-  around do |example|
-    Sequel::Model.db.transaction(rollback: :always) do
-      example.run
-    end
-  end
-
   before do
     3.times do |i|
       i += 1

spec/api/domains_api_spec.rb

@@ -5,13 +5,7 @@
   let(:admin_auth_header) { headers_for(admin_user, :admin_scope => true)["HTTP_AUTHORIZATION"] }
   authenticated_request
 
-  before do
-    reset_database
-
-    3.times do
-      VCAP::CloudController::Domain.make
-    end
-  end
+  before { 3.times { VCAP::CloudController::Domain.make } }
 
   let(:guid) { VCAP::CloudController::Domain.first.guid }
 

spec/api/service_auth_tokens_api_spec.rb

@@ -5,13 +5,7 @@
   let(:admin_auth_header) { headers_for(admin_user, :admin_scope => true)["HTTP_AUTHORIZATION"] }
   authenticated_request
 
-  before do
-    reset_database
-
-    3.times do
-      VCAP::CloudController::ServiceAuthToken.make
-    end
-  end
+  before { 3.times { VCAP::CloudController::ServiceAuthToken.make } }
 
   let(:guid) { VCAP::CloudController::ServiceAuthToken.first.guid }
 

spec/app_stager_task_spec.rb

@@ -214,7 +214,7 @@ def reply_with_staging_completion
 
           it "copes when the app is destroyed halfway between staging (currently we dont know why this happened but seen on tabasco)" do
             VCAP::CloudController::AppStagerTask::Response.stub(:new) do
-              app.destroy # We saw that app maybe destroyed half-way through staging
+              app.destroy(savepoint: true) # We saw that app maybe destroyed half-way through staging
               raise ArgumentError, "Some Fake Error"
             end
 

spec/cloud_controller_spec.rb

@@ -34,7 +34,7 @@ def self.it_creates_and_sets_admin_user
           make_request
         }.to change { user_count }.by(1)
 
-        VCAP::CloudController::User.order(:id).last.tap do |u|
+        VCAP::CloudController::User.last.tap do |u|
           expect(u.guid).to eq(user_id)
           expect(u.admin).to be_true
           expect(u.active).to be_true
@@ -109,7 +109,6 @@ def self.it_recognizes_admin_users
           before { config[:bootstrap_admin_email] = email }
 
           context "when there are 0 users in the ccdb" do
-            before { reset_database }
             it_creates_and_sets_admin_user
             it_sets_token_info
           end
@@ -125,7 +124,6 @@ def self.it_recognizes_admin_users
           before { config[:bootstrap_admin_email] = "some-other-bootstrap-email" }
 
           context "when there are 0 users in the ccdb" do
-            before { reset_database }
             it_creates_and_sets_non_admin_user
             it_sets_token_info
           end
@@ -139,7 +137,10 @@ def self.it_recognizes_admin_users
       end
 
       context "when scope includes cc admin scope" do
-        before { token_info["scope"] = [VCAP::CloudController::Roles::CLOUD_CONTROLLER_ADMIN_SCOPE] }
+        before do
+          VCAP::CloudController::User.make
+          token_info["scope"] = [VCAP::CloudController::Roles::CLOUD_CONTROLLER_ADMIN_SCOPE]
+        end
         it_creates_and_sets_admin_user
         it_sets_token_info
       end

spec/controllers/runtime/app_events_controller_spec.rb

@@ -24,8 +24,6 @@ def no_resource_has
       end
     end
 
-    before(:each) { reset_database }
-
     let(:org) { Organization.make }
     let(:space_a) { Space.make :organization => org }
 

spec/controllers/runtime/app_summaries_controller_spec.rb

@@ -2,7 +2,7 @@
 
 module VCAP::CloudController
   describe AppSummariesController, type: :controller do
-    before(:all) do
+    before do
       @num_services = 2
       @free_mem_size = 128
 
@@ -36,10 +36,6 @@ module VCAP::CloudController
       @app.add_route(@route2)
     end
 
-    after(:all) do
-      @system_domain.destroy
-    end
-
     describe "GET /v2/apps/:id/summary" do
       before do
         health_manager_client = CloudController::DependencyLocator.instance.health_manager_client

spec/controllers/runtime/apps_controller_spec.rb

@@ -485,13 +485,17 @@ def delete_app_recursively
     end
 
     describe "staging" do
-      context "when app will be staged" do
+      context "when app will be staged", non_transactional: true do
         let(:app_obj) do
           AppFactory.make(:package_hash => "abc", :state => "STOPPED",
                            :droplet_hash => nil, :package_state => "PENDING",
                            :instances => 1)
         end
 
+        after do
+          app_obj.delete
+        end
+
         it "stages the app asynchronously" do
           received_app = nil
 
@@ -537,9 +541,7 @@ def delete_app_recursively
         )
       end
 
-      before :each do
-        reset_database
-
+      before do
         user = make_developer_for_space(space)
         # keeping the headers here so that it doesn't reset the global config...
         @headers_for_user = headers_for(user)

spec/controllers/runtime/billing_events_controller_spec.rb

@@ -15,7 +15,7 @@ module VCAP::CloudController
           headers_for(user)
         end
 
-        before(:all) do
+        before do
           BillingEvent.plugin :scissors
           BillingEvent.delete
 

spec/controllers/runtime/buildpack_bits_controller_spec.rb

@@ -46,7 +46,7 @@ module VCAP::CloudController
     context "Buildpack binaries" do
       context "/v2/buildpacks/:guid/bits" do
         before { @test_buildpack = VCAP::CloudController::Buildpack.create_from_hash({ name: "upload_binary_buildpack", position: 0 }) }
-        after { @test_buildpack.destroy }
+
         let(:upload_body) { { :buildpack => valid_zip } }
 
         it "returns NOT AUTHORIZED (403) for non admins" do
@@ -138,8 +138,7 @@ module VCAP::CloudController
             })
         end
 
-        before(:all) { @test_buildpack = VCAP::CloudController::Buildpack.create_from_hash({ name: "get_binary_buildpack", key: 'xyz', position: 0 }) }
-        after(:all) { @test_buildpack.destroy }
+        before { @test_buildpack = VCAP::CloudController::Buildpack.create_from_hash({ name: "get_binary_buildpack", key: 'xyz', position: 0 }) }
 
         it "returns NOT AUTHORIZED (403) users without correct basic auth" do
           get "/v2/buildpacks/#{@test_buildpack.guid}/download", {}

spec/controllers/runtime/buildpacks_controller_spec.rb

@@ -7,8 +7,6 @@ module VCAP::CloudController
       let(:req_body) { Yajl::Encoder.encode({:name => "dynamic_test_buildpack"}) }
 
       context "POST - create a custom buildpack" do
-        after { reset_database }
-
         it "returns NOT AUTHORIZED (403) for non admins" do
           post "/v2/buildpacks", req_body, headers_for(user)
           expect(last_response.status).to eq(403)
@@ -53,8 +51,7 @@ module VCAP::CloudController
       end
 
       context "GET" do
-        before(:all) { @test_buildpack = VCAP::CloudController::Buildpack.create_from_hash({name: "get_buildpack", key: "xyz", position: 0}) }
-        after(:all) { @test_buildpack.destroy }
+        before { @test_buildpack = VCAP::CloudController::Buildpack.create_from_hash(name: "get_buildpack", key: "xyz", position: 0) }
 
         describe "/v2/buildpacks/:guid" do
           it "lets you retrieve info for a specific buildpack" do
@@ -95,10 +92,6 @@ module VCAP::CloudController
           @orig_buildpack = VCAP::CloudController::Buildpack.create_from_hash(name: "original_buildpack", key: "xyz", position: 1)
           @test_buildpack = VCAP::CloudController::Buildpack.create_from_hash(name: "update_buildpack", key: "xyz", position: 2)
         end
-        after(:each) do
-          @orig_buildpack.destroy
-          @test_buildpack.destroy
-        end
 
         it "returns NOT AUTHORIZED (403) for non admins" do
           put "/v2/buildpacks/#{@test_buildpack.guid}", {}, headers_for(user)
@@ -150,8 +143,6 @@ module VCAP::CloudController
         end
 
         context "create a default buildpack" do
-          after { @test_buildpack.destroy if @test_buildpack.exists? }
-
           it "returns NOT AUTHORIZED (403) for non admins" do
             @test_buildpack = VCAP::CloudController::Buildpack.make
             delete "/v2/buildpacks/#{@test_buildpack.guid}", {}, headers_for(user)

spec/controllers/runtime/domains_controller_spec.rb

@@ -50,17 +50,11 @@ module VCAP::CloudController
     describe "Permissions" do
       include_context "permissions"
 
-      before(:all) do
+      before do
         @system_domain = Domain.new(:name => Sham.domain,
                                             :owning_organization => nil)
         @system_domain.save(:validate => false)
-      end
 
-      after(:all) do
-        @system_domain.destroy
-      end
-
-      before do
         @obj_a = Domain.make(:owning_organization => @org_a)
         @space_a.add_domain(@obj_a)