add validation check to confirm database connection exists

Author: jbat

lib/superset/dashboard/import.rb

@@ -20,7 +20,6 @@
 module Superset
   module Dashboard
     class Import < Request
-
       attr_reader :source_zip_file, :overwrite
 
       def initialize(source_zip_file: , overwrite: true)
@@ -30,7 +29,6 @@ def initialize(source_zip_file: , overwrite: true)
 
       def perform
         validate_params
-
         response
       end
 
@@ -46,7 +44,9 @@ def response
       def validate_params
         raise ArgumentError, 'source_zip_file is required' if source_zip_file.nil?
         raise ArgumentError, 'source_zip_file does not exist' unless File.exist?(source_zip_file)
+        raise ArgumentError, 'source_zip_file is not a zip file' unless File.extname(source_zip_file) == '.zip'
         raise ArgumentError, 'overwrite must be a boolean' unless [true, false].include?(overwrite)
+        raise ArgumentError, "zip target database does not exist: #{zip_database_config_not_found_in_superset}" if zip_database_config_not_found_in_superset.present?
       end
 
       def payload
@@ -59,6 +59,26 @@ def payload
       def route
         "dashboard/import/"
       end
+
+      def zip_database_config_not_found_in_superset
+        zip_databases_details.select {|s| !superset_database_uuids_found.include?(s[:uuid]) }
+      end
+
+      def superset_database_uuids_found
+        @superset_database_uuids_found ||= begin
+          zip_databases_details.map {|i| i[:uuid]}.map do |uuid|
+            uuid if Superset::Database::List.new(uuid_equals: uuid).result.count == 1
+          end.compact
+        end
+      end
+
+      def zip_databases_details
+        zip_dashboard_config[:databases].map{|d| {uuid: d[:content][:uuid], name: d[:content][:database_name]} }
+      end
+
+      def zip_dashboard_config
+        @zip_dashboard_config ||= Superset::Services::DashboardLoader.new(dashboard_export_zip: source_zip_file).perform
+      end
     end
   end
 end

lib/superset/database/list.rb

@@ -4,10 +4,11 @@
 module Superset
   module Database
     class List < Superset::Request
-      attr_reader :title_contains
+      attr_reader :title_contains, :uuid_equals
 
-      def initialize(page_num: 0, title_contains: '')
+      def initialize(page_num: 0, title_contains: '', uuid_equals: '')
         @title_contains = title_contains
+        @uuid_equals = uuid_equals
         super(page_num: page_num)
       end
 
@@ -34,6 +35,7 @@ def filters
         # TODO filtering across all list classes can be refactored to support multiple options in a more flexible way
         filter_set = []
         filter_set << "(col:database_name,opr:ct,value:'#{title_contains}')" if title_contains.present?
+        filter_set << "(col:uuid,opr:eq,value:'#{uuid_equals}')" if uuid_equals.present?
         unless filter_set.empty?
           "filters:!(" + filter_set.join(',') + "),"
         end
@@ -45,6 +47,7 @@ def list_attributes
 
       def validate_constructor_args
         raise InvalidParameterError, "title_contains must be a String type" unless title_contains.is_a?(String)
+        raise InvalidParameterError, "uuid_equals must be a String type" unless uuid_equals.is_a?(String)
       end
     end
   end

lib/superset/request.rb

@@ -5,7 +5,6 @@ class Request
     class InvalidParameterError < StandardError; end
     class ValidationError < StandardError; end
 
-
     PAGE_SIZE = 100
 
     attr_accessor :page_num

lib/superset/services/import_dashboard_across_environment.rb

@@ -86,7 +86,6 @@ def update_dataset_configs
       def create_new_dashboard_zip
         Zip::File.open(new_zip_file, Zip::File::CREATE) do |zipfile|
           Dir[File.join(dashboard_export_root_path, '**', '**')].each do |file|
-#            puts "File Sub:" + file.sub(dashboard_export_root_path + '/', File.basename(dashboard_export_root_path) + '/' ).to_s
             zipfile.add(file.sub(dashboard_export_root_path + '/', File.basename(dashboard_export_root_path) + '/' ), file) if File.file?(file)
           end
         end

spec/superset/dashboard/import_spec.rb

@@ -14,6 +14,11 @@
 
       describe '#response' do
         context 'with valid parameters' do
+          before do
+            allow(Superset::Database::List).to receive(:new).
+              with(uuid_equals: "a2dc77af-e654-49bb-b321-40f6b559a1ee").
+              and_return(double(result: ['some data']))
+          end
 
           specify 'returns response' do
             expect(subject.perform).to eq(response)
@@ -45,6 +50,26 @@
               expect { subject.perform }.to raise_error(ArgumentError, 'overwrite must be a boolean')
             end
           end
+
+          context 'when source_zip_file is not a zip extension' do
+            let(:source_zip_file) { 'spec/fixtures/database-prod-examples.yaml' }
+
+            specify 'raises error' do
+              expect { subject.perform }.to raise_error(ArgumentError, 'source_zip_file is not a zip file')
+            end
+          end
+
+          context 'when zip_database_config_not_found_in_superset is not present' do
+            before do
+              allow(Superset::Database::List).to receive(:new).
+                with(uuid_equals: "a2dc77af-e654-49bb-b321-40f6b559a1ee").
+                and_return(double(result: []))
+            end
+
+            specify 'raises error' do
+              expect { subject.perform }.to raise_error(ArgumentError, "zip target database does not exist: [{:uuid=>\"a2dc77af-e654-49bb-b321-40f6b559a1ee\", :name=>\"examples\"}]")
+            end
+          end
         end
       end
     end

spec/superset/database/list_spec.rb

@@ -2,28 +2,30 @@
 
 RSpec.describe Superset::Database::List do
   subject { described_class.new }
-  let(:result) do
-    [
-      {
-        id: 1,
-        database_name: 'Test 1',
-        backend: 'postgres',
-        expose_in_sqllab: 'true'
-      },
-      {
-        id: 2,
-        database_name: 'Test 2',
-        backend: 'mysql',
-        expose_in_sqllab: 'false'
-      }
-    ]
-  end
-
-  before do
-    allow(subject).to receive(:result).and_return(result)
+  let(:response) do
+    { 'result' =>
+      [
+        {
+          id: 1,
+          database_name: 'Test 1',
+          backend: 'postgres',
+          expose_in_sqllab: 'true'
+        },
+        {
+          id: 2,
+          database_name: 'Test 2',
+          backend: 'mysql',
+          expose_in_sqllab: 'false'
+        }
+      ]
+    }
   end
 
   describe '#rows' do
+    before do
+      allow(subject).to receive(:response).and_return(response)
+    end
+
     specify do
       expect(subject.rows).to match_array(
         [
@@ -58,5 +60,33 @@
         expect(subject.query_params).to eq("filters:!((col:database_name,opr:ct,value:'acme')),page:0,page_size:100")
       end
     end
+
+    context 'with uuid_equals filters' do
+      subject { described_class.new(uuid_equals: '123') }
+
+      specify do
+        expect(subject.query_params).to eq("filters:!((col:uuid,opr:eq,value:'123')),page:0,page_size:100")
+      end
+    end
+  end
+
+  describe '#response' do
+    context 'with invalid parameters' do
+      context 'when title_contains is not a string' do
+        subject { described_class.new(title_contains: ['test']) }
+
+        specify do
+          expect { subject.response }.to raise_error(Superset::Request::InvalidParameterError, 'title_contains must be a String type')
+        end
+      end
+
+      context 'when uuid_equals is not a string' do
+        subject { described_class.new(uuid_equals: 1) }
+
+        specify do
+          expect { subject.response }.to raise_error(Superset::Request::InvalidParameterError, 'uuid_equals must be a String type')
+        end
+      end
+    end
   end
 end