Merge pull request #40 from rdytech/NEP-17596_export_dashboards_should_save_zip_file_and_clean_out_prev_ver

[NEP-17596] Superset: Export Dashboards should save zip file and clean out prev version totally

Author: jbat

lib/superset/dashboard/export.rb

@@ -23,10 +23,18 @@ def initialize(dashboard_id: , destination_path: )
       end
 
       def perform
+        logger.info("Exporting dashboard: #{dashboard_id}")
         create_tmp_dir
         save_exported_zip_file
         unzip_files
+        clean_destination_directory
         copy_export_files_to_destination_path if destination_path
+
+        Dir.glob("#{destination_path_with_dash_id}/**/*").select { |f| File.file?(f) }
+      rescue StandardError => e
+        raise
+      ensure
+        cleanup_temp_dir
       end
 
       def response
@@ -37,43 +45,67 @@ def response
         )
       end
 
+      def zip_file_name
+        @zip_file_name ||= "#{tmp_uniq_dashboard_path}/dashboard_#{dashboard_id}_export_#{datestamp}.zip"
+      end
+
       private
 
       def params
         { "q": "!(#{dashboard_id})" }   # pulled off chrome dev tools doing a GUI export.  Swagger interface not helpfull with this endpoint.
       end
 
       def save_exported_zip_file
+        logger.info("Saving zip file: #{zip_file_name}")
         File.open(zip_file_name, 'wb') { |fp| fp.write(response.body) }
       end
 
       def unzip_files
+        logger.info("Unzipping file: #{zip_file_name}")
         @extracted_files = unzip_file(zip_file_name, tmp_uniq_dashboard_path)
       end
 
       def download_folder
         File.dirname(extracted_files[0])
       end
 
+      def destination_path_with_dash_id
+        @destination_path_with_dash_id ||= File.join(destination_path, dashboard_id.to_s)
+      end
+
+      def clean_destination_directory
+        logger.info("Cleaning destination directory: #{destination_path_with_dash_id}")
+        if Dir.exist?(destination_path_with_dash_id)
+          FileUtils.rm_rf(Dir.glob("#{destination_path_with_dash_id}/*"))
+        else
+          FileUtils.mkdir_p(destination_path_with_dash_id)
+        end
+      end
+
       def copy_export_files_to_destination_path
+        logger.info("Copying files to destination: #{destination_path_with_dash_id}")
         path_with_dash_id = File.join(destination_path, dashboard_id.to_s)
         FileUtils.mkdir_p(path_with_dash_id) unless File.directory?(path_with_dash_id)
+        FileUtils.cp(zip_file_name, path_with_dash_id)
 
         Dir.glob("#{download_folder}/*").each do |item|
           FileUtils.cp_r(item, path_with_dash_id)
         end
       end
 
-      def zip_file_name
-        @zip_file_name ||= "#{tmp_uniq_dashboard_path}/dashboard_#{dashboard_id}_export_#{datestamp}.zip"
+      def cleanup_temp_dir
+        if Dir.exist?(tmp_uniq_dashboard_path)
+          FileUtils.rm_rf(tmp_uniq_dashboard_path)
+        end
       end
 
       def create_tmp_dir
+        logger.info("Creating tmp directory: #{tmp_uniq_dashboard_path}")
         FileUtils.mkdir_p(tmp_uniq_dashboard_path) unless File.directory?(tmp_uniq_dashboard_path)
       end
 
       # uniq random tmp folder name for each export
-      # this will allow us to do a wildcard glop on the folder to get the files 
+      # this will allow us to do a wildcard glop on the folder to get the files
       def tmp_uniq_dashboard_path
         @tmp_uniq_dashboard_path ||= File.join(TMP_SUPERSET_DASHBOARD_PATH, uuid)
       end
@@ -93,6 +125,25 @@ def route
       def datestamp
         @datestamp ||= Time.now.strftime('%Y%m%d')
       end
+
+      def unzip_file(zip_path, destination)
+        extracted_files = []
+        Zip::File.open(zip_path) do |zip_file|
+          zip_file.each do |entry|
+            entry_path = File.join(destination, entry.name)
+            FileUtils.mkdir_p(File.dirname(entry_path))
+            zip_file.extract(entry, entry_path) unless File.exist?(entry_path)
+            extracted_files << entry_path
+          end
+        end
+        extracted_files
+      rescue => e
+        raise
+      end
+    end
+
+    def logger
+      @logger ||= Superset::Logger.new
     end
   end
 end

spec/superset/dashboard/export_spec.rb

@@ -2,23 +2,55 @@
 
 RSpec.describe Superset::Dashboard::Export do
   subject { described_class.new(dashboard_id: dashboard_id, destination_path: destination_path) }
+
   let(:dashboard_id) { 18 }
   let(:destination_path) { './tmp/superset_dashboard_backups/' }
+  let(:datestamp) { '20240322' }
+  let(:zip_file_name) { 'spec/fixtures/dashboard_18_export_20240322.zip' } # Example fixture zip file
 
   describe '#perform' do
     let(:response) { double('response') }
-    let(:zip_file_name) { 'spec/fixtures/dashboard_18_export_20240322.zip' } # example birth names dashboard export fixture
 
     before do
+      FileUtils.rm_rf("#{destination_path}#{dashboard_id}") if File.directory?("#{destination_path}#{dashboard_id}")
+
       allow(subject).to receive(:response).and_return(response)
       allow(subject).to receive(:zip_file_name).and_return(zip_file_name)
       allow(subject).to receive(:save_exported_zip_file)
+
+      allow(subject).to receive(:datestamp).and_return(datestamp)
+
+      @temp_dir = Dir.mktmpdir("superset_dashboard_exports")
+      @extracted_files = [
+        File.join(@temp_dir, "dashboard_export_#{datestamp}T123456", "metadata.yaml"),
+        File.join(@temp_dir, "dashboard_export_#{datestamp}T123456", "dashboards", "Birth_Names_18.yaml"),
+        File.join(@temp_dir, "dashboard_export_#{datestamp}T123456", "charts", "Boy_Name_Cloud_53920.yaml"),
+        File.join(@temp_dir, "dashboard_export_#{datestamp}T123456", "charts", "Names_Sorted_by_Num_in_California_53929.yaml"),
+        File.join(@temp_dir, "dashboard_export_#{datestamp}T123456", "charts", "Number_of_Girls_53930.yaml"),
+        File.join(@temp_dir, "dashboard_export_#{datestamp}T123456", "charts", "Pivot_Table_53931.yaml"),
+        File.join(@temp_dir, "dashboard_export_#{datestamp}T123456", "charts", "Top_10_Girl_Name_Share_53921.yaml"),
+        File.join(@temp_dir, "dashboard_export_#{datestamp}T123456", "databases", "examples.yaml"),
+        File.join(@temp_dir, "dashboard_export_#{datestamp}T123456", "datasets", "examples", "video_game_sales.yaml")
+      ]
+
+      FileUtils.mkdir_p(File.dirname(@extracted_files.first)) # Creates dashboard_export_20240322T123456
+      @extracted_files.each do |file_path|
+        FileUtils.mkdir_p(File.dirname(file_path))
+        File.write(file_path, "dummy content for #{File.basename(file_path)}")
+      end
+
+      allow(subject).to receive(:unzip_file).and_return(@extracted_files)
     end
 
-    it 'unzips into the destination path' do
+    after do
+      FileUtils.rm_rf(@temp_dir) if Dir.exist?(@temp_dir)
+      FileUtils.rm_rf("#{destination_path}#{dashboard_id}") if File.directory?("#{destination_path}#{dashboard_id}")
+    end
+
+    it 'unzips into the destination path with versioned filenames' do
       subject.perform
-      expect(Dir.glob(subject.destination_path + "/**/*").sort).to match_array([
-        "./tmp/superset_dashboard_backups/18",
+
+      expected_files = [
         "./tmp/superset_dashboard_backups/18/charts",
         "./tmp/superset_dashboard_backups/18/charts/Boy_Name_Cloud_53920.yaml",
         "./tmp/superset_dashboard_backups/18/charts/Names_Sorted_by_Num_in_California_53929.yaml",
@@ -31,9 +63,18 @@
         "./tmp/superset_dashboard_backups/18/databases/examples.yaml",
         "./tmp/superset_dashboard_backups/18/datasets",
         "./tmp/superset_dashboard_backups/18/datasets/examples",
-        "./tmp/superset_dashboard_backups/18/datasets/examples/birth_names.yaml",
-        "./tmp/superset_dashboard_backups/18/metadata.yaml"
-      ])
+        "./tmp/superset_dashboard_backups/18/datasets/examples/video_game_sales.yaml",
+        "./tmp/superset_dashboard_backups/18/metadata.yaml",
+        "./tmp/superset_dashboard_backups/18/dashboard_18_export_20240322.zip" # Include the zip file itself
+      ]
+
+      actual_files = Dir.glob("#{destination_path}#{dashboard_id}/**/*").sort.map do |path|
+        Pathname.new(path).relative_path_from(Pathname.new('.')).to_s
+      end
+
+      actual_files = actual_files.map { |path| "./#{path}" }
+
+      expect(actual_files).to match_array(expected_files)
     end
   end
-end
+end