Merge pull request #16315 from CartoDB/bug/ch89835/inter-cloud-migrations-may-leave-redis-keys

[ch89835] Migrate DO subscription information in inter-cloud migrations

NEWS.md

@@ -21,6 +21,7 @@ Development
 - Use the organization user's data while editing a user from organization settings [#16280](https://github.com/CartoDB/cartodb/pull/16280)
 - Fix schema name in layers created by free users [#16307](https://github.com/CartoDB/cartodb/pull/16307)
 - Limit start parameter of Dropbox connector [#16264](https://github.com/CartoDB/cartodb/pull/16264)
+- Migrate Redis DO subscription information in inter-cloud migrations [#16315](https://github.com/CartoDB/cartodb/pull/16315)
 - OauthApps restricted by default [#16304](https://github.com/CartoDB/cartodb/pull/16304)
 - Support staging hostname in the catalog [#16258](https://github.com/CartoDB/cartodb/pull/16258)
 - Fix user migration export/import logs [#16298](https://github.com/CartoDB/cartodb/pull/16298)

app/services/carto/organization_metadata_export_service.rb

@@ -316,6 +316,13 @@ module Carto
         Carto::UserMetadataExportService.new.import_search_tweets_from_directory(user, "#{path}/user_#{user.id}")
       end
 
+      organization.users.each do |user|
+        Carto::UserMetadataExportService.new.import_redis_do_subscriptions(
+          user,
+          "#{path}/user_#{user.id}"
+        )
+      end
+
       organization
     end
   end

app/services/carto/redis_export_service.rb

@@ -2,9 +2,11 @@ require 'json'
 
 # Version History
 # 1.0.0: export organization metadata
+# 1.0.1: add DO subscriptions
 module Carto
   module RedisExportServiceConfiguration
-    CURRENT_VERSION = '1.0.0'.freeze
+
+    CURRENT_VERSION = '1.0.1'.freeze
 
     def compatible_version?(version)
       version.to_i == CURRENT_VERSION.split('.')[0].to_i
@@ -34,6 +36,13 @@ module Carto
       remove_redis(exported_hash[:redis])
     end
 
+    def restore_redis_do_subscriptions_from_json_export(exported_json_string, user)
+      exported_hash = JSON.parse(exported_json_string).deep_symbolize_keys
+      raise 'Wrong export version' unless compatible_version?(exported_hash[:version])
+
+      restore_do_subscriptions($users_metadata, exported_hash[:redis][:do_subscriptions], user)
+    end
+
     private
 
     def restore_redis(redis_export)
@@ -44,6 +53,7 @@ module Carto
     def remove_redis(redis_export)
       remove_keys($users_metadata, redis_export[:users_metadata])
       remove_keys($tables_metadata, redis_export[:tables_metadata])
+      remove_keys($users_metadata, redis_export[:do_subscriptions])
     end
 
     def restore_keys(redis_db, redis_keys)
@@ -60,6 +70,22 @@ module Carto
       end
     end
 
+    def restore_do_subscriptions(redis_db, redis_keys, user)
+      redis_keys.each do |key, value|
+        subscriptions = JSON.parse(value.presence || '[]')
+
+        subscriptions.each do |sub|
+          dataset = user.tables.find_by(name: sub['sync_table'])
+          next if dataset.nil?
+
+          sub['sync_table_id'] = dataset.id
+          sub['synchronization_id'] = dataset.synchronization.try(:id)
+        end
+
+        redis_db.hset(key, Carto::DoLicensingService::PRESELECTED_STORAGE, subscriptions.to_json)
+      end
+    end
+
     def remove_keys(redis_db, redis_keys)
       redis_keys.each do |key|
         redis_db.del(key)
@@ -97,14 +123,16 @@ module Carto
     def export_organization(organization)
       {
         users_metadata: export_dataservices("org:#{organization.name}"),
-        tables_metadata: {}
+        tables_metadata: {},
+        do_subscriptions: {}
       }
     end
 
     def export_user(user)
       {
         users_metadata: export_dataservices("user:#{user.username}"),
-        tables_metadata: export_named_maps(user)
+        tables_metadata: export_named_maps(user),
+        do_subscriptions: export_do_subscriptions(user)
       }
     end
 
@@ -126,6 +154,17 @@ module Carto
       { named_maps_key => named_maps_hash }
     end
 
+    def export_do_subscriptions(user)
+      do_subscriptions_key = "do:#{user.username}:datasets"
+      do_subscriptions =
+        $users_metadata.hget(do_subscriptions_key, Carto::DoLicensingService::PRESELECTED_STORAGE)
+      return {} if do_subscriptions.nil?
+
+      {
+        do_subscriptions_key => do_subscriptions
+      }
+    end
+
     def matches_user_visualization?(named_map, user)
       re = /tpl_(?<viz_id>.+)/
       match = re.match(named_map)

app/services/carto/user_metadata_export_service.rb

@@ -573,6 +573,8 @@ module Carto
       import_user_visualizations_from_directory(user, Carto::Visualization::TYPE_DERIVED, meta_path)
 
       import_search_tweets_from_directory(user, meta_path)
+
+      import_redis_do_subscriptions(user, meta_path)
     end
 
     def import_search_tweets_from_directory(user, meta_path)
@@ -581,6 +583,13 @@ module Carto
       search_tweets.each { |st| save_imported_search_tweet(st, user) }
     end
 
+    def import_redis_do_subscriptions(user, meta_path)
+      Carto::RedisExportService.new.restore_redis_do_subscriptions_from_json_export(
+        redis_user_file(meta_path),
+        user
+      )
+    end
+
     private
 
     def user_from_file(path)

script/ci/cloudbuild-onprem-dev-tests.yaml

@@ -365,7 +365,7 @@ steps:
         fi
   waitFor: ['upload-logs-onprem']
 
-timeout: 1800s
+timeout: 2100s
 options:
   machineType: 'E2_HIGHCPU_32'
 substitutions:

spec/services/carto/redis_export_service_spec.rb

@@ -31,7 +31,7 @@ describe Carto::RedisExportService do
     table_visualization.save!
     visualization.save!
 
-    yield(visualization)
+    yield(visualization, table_visualization)
   ensure
     map.destroy
     table.destroy
@@ -48,6 +48,33 @@ describe Carto::RedisExportService do
     $tables_metadata.del("map_tpl|#{visualization.user.username}")
   end
 
+  def with_synchronization(visualization)
+    synchronization = create(:carto_synchronization, visualization: visualization)
+
+    yield(synchronization)
+  ensure
+    synchronization.destroy
+  end
+
+  def with_do_subscription(synchronization)
+    redis_key = "do:#{@user.username}:datasets"
+    $users_metadata.hset(redis_key, 'bq', [{
+      dataset_id: 'dataset-id',
+      status: 'active',
+      available_in: ['bq', 'bq-sample'],
+      license_type: 'full-access',
+      type: 'dataset',
+      sync_status: 'synced',
+      sync_table: synchronization.visualization.user_table.name,
+      sync_table_id: synchronization.visualization.user_table.id,
+      synchronization_id: synchronization.id
+    }].to_json)
+
+    yield
+  ensure
+    $users_metadata.del(redis_key)
+  end
+
   def check_export(export, prefix)
     expect(export[:redis][:users_metadata].keys.sort).to eq(["#{prefix}:hash", "#{prefix}:set", "#{prefix}:string"])
   end
@@ -57,6 +84,15 @@ describe Carto::RedisExportService do
     expect(export[:redis][:tables_metadata]["map_tpl|#{visualization.user.username}"]['custom_named']).to eq(Base64.encode64("{:c=>\"custom\"}"))
   end
 
+  def check_do_subscriptions(export, synchronization)
+    expect(export[:redis][:do_subscriptions].keys).to eq(["do:#{@user.username}:datasets"])
+    expect(
+      JSON.parse(
+        export[:redis][:do_subscriptions]["do:#{@user.username}:datasets"]
+      ).first['synchronization_id']
+    ).to eq(synchronization.id)
+  end
+
   def check_redis(prefix)
     expect($users_metadata.get("#{prefix}:string")).to eq('something')
     expect($users_metadata.zrange("#{prefix}:set", 0, -1, withscores: true)).to eq([['set_key', 5.0]])
@@ -67,6 +103,13 @@ describe Carto::RedisExportService do
     expect($tables_metadata.hgetall("map_tpl|#{@user.username}")). to eq("custom_named" => "{:c=>\"custom\"}")
   end
 
+  def check_redis_do_subscriptions
+    expect($users_metadata.hget("do:#{@user.username}:datasets", 'bq').present?).to be_true
+    expect(
+      JSON.parse($users_metadata.hget("do:#{@user.username}:datasets", 'bq')).first['dataset_id']
+    ).to eq('dataset-id')
+  end
+
   let(:service) { Carto::RedisExportService.new }
 
   describe '#export' do
@@ -93,13 +136,24 @@ describe Carto::RedisExportService do
     end
 
     it 'includes non-cartodb-managed named maps' do
-      with_visualization do |visualization|
+      with_visualization do |visualization, _|
         with_named_maps(visualization) do
           export = service.export_user_json_hash(@user)
           check_named_maps(export, visualization)
         end
       end
     end
+
+    it 'includes DO subscriptions' do
+      with_visualization do |_, table_visualization|
+        with_synchronization(table_visualization) do |synchronization|
+          with_do_subscription(synchronization) do
+            export = service.export_user_json_hash(@user)
+            check_do_subscriptions(export, synchronization)
+          end
+        end
+      end
+    end
   end
 
   describe '#export + import' do
@@ -111,7 +165,7 @@ describe Carto::RedisExportService do
     end
 
     it 'copies all keys under user:username for users' do
-      with_visualization do |visualization|
+      with_visualization do |visualization, _|
         prefix = "user:#{@user.username}"
         export = with_redis_keys(prefix) do
           with_named_maps(visualization) do
@@ -123,5 +177,18 @@ describe Carto::RedisExportService do
         check_redis_named_maps
       end
     end
+
+    it 'copies DO subscriptions' do
+      with_visualization do |_, table_visualization|
+        with_synchronization(table_visualization) do |synchronization|
+          export = with_do_subscription(synchronization) do
+            service.export_user_json_hash(@user)
+          end
+
+          service.restore_redis_do_subscriptions_from_json_export(export.to_json, @user)
+          check_redis_do_subscriptions
+        end
+      end
+    end
   end
 end

spec/services/carto/user_metadata_export_service_spec.rb

@@ -103,6 +103,19 @@ describe Carto::UserMetadataExportService do
     create(:oauth_access_tokens, oauth_app_user: oauth_app_user, api_key: api_key)
     create(:oauth_refresh_tokens, oauth_app_user: oauth_app_user, scopes: ['offline'])
 
+    # DO subscriptions
+    $users_metadata.hset("do:#{@user.username}:datasets", 'bq', [{
+      dataset_id: 'dataset-id',
+      status: 'active',
+      available_in: ['bq', 'bq-sample'],
+      license_type: 'full-access',
+      type: 'dataset',
+      sync_status: 'synced',
+      sync_table: @table.name,
+      sync_table_id: @table.id,
+      synchronization_id: sync.id
+    }].to_json)
+
     @user.reload
   end
 
@@ -497,6 +510,12 @@ describe Carto::UserMetadataExportService do
 
   def expect_redis_restored(user)
     expect(CartoDB::GeocoderUsageMetrics.new(user.username).get(:geocoder_here, :success_responses)).to eq(1)
+
+    expect(user.subscriptions.count).to eq(1)
+    expect(user.subscriptions.first['sync_table']).to eq(@table.name)
+    if user.tables.exists?(name: user.subscriptions.first['sync_table'])
+      expect(user.subscriptions.first['sync_table_id']).not_to eq(@table.id)
+    end
   end
 
   def expect_export_matches_search_tweet(exported_search_tweet, search_tweet)
@@ -745,6 +764,9 @@ describe Carto::UserMetadataExportService do
     $tables_metadata.del(Carto::Visualization::V2_VISUALIZATIONS_REDIS_KEY)
     expect(@visualization.uses_vizjson2?).to be_false
 
+    # Clean redis DO subscriptions
+    $tables_metadata.del("do:#{@user.username}:datasets", 'bq')
+
     @imported_user = service.import_from_directory(path)
     service.import_metadata_from_directory(@imported_user, path)