REFACTOR: admin backups controller specs to requests (#5953)

spec/controllers/admin/backups_controller_spec.rb

@@ -1,264 +0,0 @@
-require "rails_helper"
-
-describe Admin::BackupsController do
-
-  it "is a subclass of AdminController" do
-    expect(Admin::BackupsController < Admin::AdminController).to eq(true)
-  end
-
-  let(:backup_filename) { "2014-02-10-065935.tar.gz" }
-
-  context "while logged in as an admin" do
-
-    before { @admin = log_in(:admin) }
-
-    describe ".index" do
-
-      context "html format" do
-
-        it "preloads important data" do
-          Backup.expects(:all).returns([])
-          subject.expects(:store_preloaded).with("backups", "[]")
-
-          BackupRestore.expects(:operations_status).returns({})
-          subject.expects(:store_preloaded).with("operations_status", "{}")
-
-          BackupRestore.expects(:logs).returns([])
-          subject.expects(:store_preloaded).with("logs", "[]")
-
-          get :index, format: :html, xhr: true
-
-          expect(response.status).to eq(200)
-        end
-
-      end
-
-      context "json format" do
-
-        it "returns a list of all the backups" do
-          Backup.expects(:all).returns([Backup.new("backup1"), Backup.new("backup2")])
-
-          get :index, format: :json, xhr: true
-
-          expect(response.status).to eq(200)
-
-          json = JSON.parse(response.body)
-          expect(json[0]["filename"]).to eq("backup1")
-          expect(json[1]["filename"]).to eq("backup2")
-        end
-
-      end
-
-    end
-
-    describe ".status" do
-
-      it "returns the current backups status" do
-        BackupRestore.expects(:operations_status)
-
-        get :status, format: :json
-
-        expect(response.status).to eq(200)
-      end
-
-    end
-
-    describe ".create" do
-
-      it "starts a backup" do
-        BackupRestore.expects(:backup!).with(@admin.id, publish_to_message_bus: true, with_uploads: false, client_id: "foo")
-
-        post :create, params: {
-          with_uploads: false, client_id: "foo"
-        }, format: :json
-
-        expect(response.status).to eq(200)
-      end
-
-    end
-
-    describe ".show" do
-
-      it "uses send_file to transmit the backup" do
-        begin
-          token = EmailBackupToken.set(@admin.id)
-          path = File.join(Backup.base_directory, backup_filename)
-          File.open(path, "w") { |f| f.write("hello") }
-
-          Backup.create_from_filename(backup_filename)
-
-          StaffActionLogger.any_instance.expects(:log_backup_download).once
-
-          get :show, params: { id: backup_filename, token: token }, format: :json
-
-          expect(response.headers['Content-Length']).to eq("5")
-          expect(response.headers['Content-Disposition']).to match(/attachment; filename/)
-        ensure
-          File.delete(path)
-          EmailBackupToken.del(@admin.id)
-        end
-      end
-
-      it "returns 422 when token is bad" do
-        begin
-          path = File.join(Backup.base_directory, backup_filename)
-          File.open(path, "w") { |f| f.write("hello") }
-
-          Backup.create_from_filename(backup_filename)
-
-          get :show, params: { id: backup_filename, token: "bad_value" }, xhr: true
-
-          expect(response.status).to eq(422)
-        ensure
-          File.delete(path)
-        end
-      end
-
-      it "returns 404 when the backup does not exist" do
-        token = EmailBackupToken.set(@admin.id)
-        Backup.expects(:[]).returns(nil)
-
-        get :show, params: { id: backup_filename, token: token }, format: :json
-
-        EmailBackupToken.del(@admin.id)
-
-        expect(response).to be_not_found
-      end
-
-    end
-
-    describe ".destroy" do
-
-      let(:b) { Backup.new(backup_filename) }
-
-      it "removes the backup if found" do
-        Backup.expects(:[]).with(backup_filename).returns(b)
-        b.expects(:remove)
-
-        StaffActionLogger.any_instance.expects(:log_backup_destroy).with(b).once
-
-        delete :destroy, params: { id: backup_filename }, format: :json
-
-        expect(response.status).to eq(200)
-      end
-
-      it "doesn't remove the backup if not found" do
-        Backup.expects(:[]).with(backup_filename).returns(nil)
-        b.expects(:remove).never
-        delete :destroy, params: { id: backup_filename }, format: :json
-        expect(response).not_to be_successful
-      end
-
-    end
-
-    describe ".logs" do
-
-      it "preloads important data" do
-        BackupRestore.expects(:operations_status).returns({})
-        subject.expects(:store_preloaded).with("operations_status", "{}")
-
-        BackupRestore.expects(:logs).returns([])
-        subject.expects(:store_preloaded).with("logs", "[]")
-
-        get :logs, format: :html, xhr: true
-
-        expect(response.status).to eq(200)
-      end
-    end
-
-    describe ".restore" do
-
-      it "starts a restore" do
-        expect(SiteSetting.disable_emails).to eq("no")
-        BackupRestore.expects(:restore!).with(@admin.id, filename: backup_filename, publish_to_message_bus: true, client_id: "foo")
-
-        post :restore, params: { id: backup_filename, client_id: "foo" }, format: :json
-
-        expect(SiteSetting.disable_emails).to eq("yes")
-        expect(response.status).to eq(200)
-      end
-
-    end
-
-    describe ".readonly" do
-
-      it "enables readonly mode" do
-        Discourse.expects(:enable_readonly_mode)
-
-        expect { put :readonly, params: { enable: true }, format: :json }
-          .to change { UserHistory.count }.by(1)
-
-        expect(response.status).to eq(200)
-
-        user_history = UserHistory.last
-
-        expect(UserHistory.last.action).to eq(UserHistory.actions[:change_readonly_mode])
-        expect(UserHistory.last.new_value).to eq('t')
-      end
-
-      it "disables readonly mode" do
-        Discourse.expects(:disable_readonly_mode)
-
-        expect { put :readonly, params: { enable: false }, format: :json }
-          .to change { UserHistory.count }.by(1)
-
-        expect(response.status).to eq(200)
-
-        user_history = UserHistory.last
-
-        expect(UserHistory.last.action).to eq(UserHistory.actions[:change_readonly_mode])
-        expect(UserHistory.last.new_value).to eq('f')
-      end
-
-    end
-
-    describe "#upload_backup_chunk" do
-      describe "when filename contains invalid characters" do
-        it "should raise an error" do
-          ['灰色.tar.gz', '; echo \'haha\'.tar.gz'].each do |invalid_filename|
-            described_class.any_instance.expects(:has_enough_space_on_disk?).returns(true)
-
-            post :upload_backup_chunk, params: {
-              resumableFilename: invalid_filename, resumableTotalSize: 1
-            }
-
-            expect(response.status).to eq(415)
-            expect(response.body).to eq(I18n.t('backup.invalid_filename'))
-          end
-        end
-      end
-
-      describe "when filename is valid" do
-        it "should upload the file successfully" do
-          begin
-            described_class.any_instance.expects(:has_enough_space_on_disk?).returns(true)
-
-            filename = 'test_Site-0123456789.tar.gz'
-
-            post :upload_backup_chunk, params: {
-              resumableFilename: filename,
-              resumableTotalSize: 1,
-              resumableIdentifier: 'test',
-              resumableChunkNumber: '1',
-              resumableChunkSize: '1',
-              resumableCurrentChunkSize: '1',
-              file: fixture_file_upload(Tempfile.new)
-            }, format: :json
-
-            expect(response.status).to eq(200)
-            expect(response.body).to eq("")
-          ensure
-            begin
-              File.delete(
-                File.join(Backup.base_directory, 'tmp', 'test', "#{filename}.part1")
-              )
-            rescue Errno::ENOENT
-            end
-          end
-        end
-      end
-    end
-
-  end
-
-end

spec/requests/admin/backups_controller_spec.rb

@@ -2,6 +2,12 @@ require 'rails_helper'
 
 RSpec.describe Admin::BackupsController do
   let(:admin) { Fabricate(:admin) }
+  let(:backup_filename) { "2014-02-10-065935.tar.gz" }
+  let(:backup_filename2) { "2014-02-11-065935.tar.gz" }
+
+  it "is a subclass of AdminController" do
+    expect(Admin::BackupsController < Admin::AdminController).to eq(true)
+  end
 
   before do
     sign_in(admin)
@@ -11,7 +17,235 @@ RSpec.describe Admin::BackupsController do
     it "raises an error when backups are disabled" do
       SiteSetting.enable_backups = false
       get "/admin/backups.json"
-      expect(response).not_to be_successful
+      expect(response.status).to eq(403)
+    end
+
+    context "html format" do
+      it "preloads important data" do
+        get "/admin/backups.html"
+        expect(response.status).to eq(200)
+
+        preloaded = controller.instance_variable_get("@preloaded").map do |key, value|
+          [key, JSON.parse(value)]
+        end.to_h
+
+        expect(preloaded["backups"].size).to eq(Backup.all.size)
+        expect(preloaded["operations_status"].symbolize_keys).to eq(BackupRestore.operations_status)
+        expect(preloaded["logs"].size).to eq(BackupRestore.logs.size)
+      end
+    end
+
+    context "json format" do
+      it "returns a list of all the backups" do
+        begin
+          paths = []
+          [backup_filename, backup_filename2].each do |name|
+            path = File.join(Backup.base_directory, name)
+            paths << path
+            File.open(path, "w") { |f| f.write("hello") }
+            Backup.create_from_filename(name)
+          end
+
+          get "/admin/backups.json"
+
+          expect(response.status).to eq(200)
+
+          json = JSON.parse(response.body).map { |backup| backup["filename"] }
+          expect(json).to include(backup_filename)
+          expect(json).to include(backup_filename2)
+        ensure
+          paths.each { |path| File.delete(path) }
+        end
+      end
+    end
+  end
+
+  describe '#status' do
+    it "returns the current backups status" do
+      get "/admin/backups/status.json"
+      expect(response.body).to eq(BackupRestore.operations_status.to_json)
+      expect(response.status).to eq(200)
+    end
+  end
+
+  describe '#create' do
+    it "starts a backup" do
+      BackupRestore.expects(:backup!).with(admin.id, publish_to_message_bus: true, with_uploads: false, client_id: "foo")
+
+      post "/admin/backups.json", params: {
+        with_uploads: false, client_id: "foo"
+      }
+
+      expect(response.status).to eq(200)
+    end
+  end
+
+  describe '#show' do
+    it "uses send_file to transmit the backup" do
+      begin
+        token = EmailBackupToken.set(admin.id)
+        path = File.join(Backup.base_directory, backup_filename)
+        File.open(path, "w") { |f| f.write("hello") }
+
+        Backup.create_from_filename(backup_filename)
+
+        expect do
+          get "/admin/backups/#{backup_filename}.json", params: { token: token }
+        end.to change { UserHistory.where(action: UserHistory.actions[:backup_download]).count }.by(1)
+
+        expect(response.headers['Content-Length']).to eq("5")
+        expect(response.headers['Content-Disposition']).to match(/attachment; filename/)
+      ensure
+        File.delete(path)
+        EmailBackupToken.del(admin.id)
+      end
+    end
+
+    it "returns 422 when token is bad" do
+      begin
+        path = File.join(Backup.base_directory, backup_filename)
+        File.open(path, "w") { |f| f.write("hello") }
+
+        Backup.create_from_filename(backup_filename)
+
+        get "/admin/backups/#{backup_filename}.json", params: { token: "bad_value" }
+
+        expect(response.status).to eq(422)
+        expect(response.headers['Content-Disposition']).not_to match(/attachment; filename/)
+      ensure
+        File.delete(path)
+      end
+    end
+
+    it "returns 404 when the backup does not exist" do
+      token = EmailBackupToken.set(admin.id)
+      get "/admin/backups/#{backup_filename}.json", params: { token: token }
+
+      EmailBackupToken.del(admin.id)
+      expect(response.status).to eq(404)
+    end
+  end
+
+  describe '#destroy' do
+    let(:b) { Backup.new(backup_filename) }
+
+    it "removes the backup if found" do
+      begin
+        path = File.join(Backup.base_directory, backup_filename)
+        File.open(path, "w") { |f| f.write("hello") }
+
+        Backup.create_from_filename(backup_filename)
+
+        expect do
+          delete "/admin/backups/#{backup_filename}.json"
+        end.to change { UserHistory.where(action: UserHistory.actions[:backup_destroy]).count }.by(1)
+
+        expect(response.status).to eq(200)
+        expect(File.exists?(path)).to eq(false)
+      ensure
+        File.delete(path) if File.exists?(path)
+      end
+    end
+
+    it "doesn't remove the backup if not found" do
+      delete "/admin/backups/#{backup_filename}.json"
+      expect(response.status).to eq(404)
+    end
+  end
+
+  describe '#logs' do
+    it "preloads important data" do
+      get "/admin/backups/logs.html"
+      expect(response.status).to eq(200)
+
+      preloaded = controller.instance_variable_get("@preloaded").map do |key, value|
+        [key, JSON.parse(value)]
+      end.to_h
+
+      expect(preloaded["operations_status"].symbolize_keys).to eq(BackupRestore.operations_status)
+      expect(preloaded["logs"].size).to eq(BackupRestore.logs.size)
+    end
+  end
+
+  describe '#restore' do
+    it "starts a restore" do
+      expect(SiteSetting.disable_emails).to eq("no")
+      BackupRestore.expects(:restore!).with(admin.id, filename: backup_filename, publish_to_message_bus: true, client_id: "foo")
+
+      post "/admin/backups/#{backup_filename}/restore.json", params: { client_id: "foo" }
+
+      expect(SiteSetting.disable_emails).to eq("yes")
+      expect(response.status).to eq(200)
+    end
+  end
+
+  describe '#readonly' do
+    it "enables readonly mode" do
+      expect(Discourse.readonly_mode?).to eq(false)
+
+      expect { put "/admin/backups/readonly.json", params: { enable: true } }
+        .to change { UserHistory.where(action: UserHistory.actions[:change_readonly_mode], new_value: "t").count }.by(1)
+
+      expect(Discourse.readonly_mode?).to eq(true)
+      expect(response.status).to eq(200)
+    end
+
+    it "disables readonly mode" do
+      Discourse.enable_readonly_mode(Discourse::USER_READONLY_MODE_KEY)
+      expect(Discourse.readonly_mode?).to eq(true)
+
+      expect { put "/admin/backups/readonly.json", params: { enable: false } }
+        .to change { UserHistory.where(action: UserHistory.actions[:change_readonly_mode], new_value: "f").count }.by(1)
+
+      expect(response.status).to eq(200)
+      expect(Discourse.readonly_mode?).to eq(false)
+    end
+  end
+
+  describe "#upload_backup_chunk" do
+    describe "when filename contains invalid characters" do
+      it "should raise an error" do
+        ['灰色.tar.gz', '; echo \'haha\'.tar.gz'].each do |invalid_filename|
+          described_class.any_instance.expects(:has_enough_space_on_disk?).returns(true)
+
+          post "/admin/backups/upload", params: {
+            resumableFilename: invalid_filename, resumableTotalSize: 1
+          }
+
+          expect(response.status).to eq(415)
+          expect(response.body).to eq(I18n.t('backup.invalid_filename'))
+        end
+      end
+    end
+
+    describe "when filename is valid" do
+      it "should upload the file successfully" do
+        begin
+          described_class.any_instance.expects(:has_enough_space_on_disk?).returns(true)
+
+          filename = 'test_Site-0123456789.tar.gz'
+
+          post "/admin/backups/upload.json", params: {
+            resumableFilename: filename,
+            resumableTotalSize: 1,
+            resumableIdentifier: 'test',
+            resumableChunkNumber: '1',
+            resumableChunkSize: '1',
+            resumableCurrentChunkSize: '1',
+            file: fixture_file_upload(Tempfile.new)
+          }
+
+          expect(response.status).to eq(200)
+          expect(response.body).to eq("")
+        ensure
+          begin
+            File.delete(
+              File.join(Backup.base_directory, 'tmp', 'test', "#{filename}.part1")
+            )
+          rescue Errno::ENOENT
+          end
+        end
+      end
     end
   end
 
@@ -67,6 +301,5 @@ RSpec.describe Admin::BackupsController do
 
       expect(response).to be_not_found
     end
-
   end
 end