Browse Source

Improved initial import and sync of translations (about 5-8 times faster).

Martin Edenhofer 6 years ago
parent
commit
5bfa535b62
4 changed files with 121 additions and 41 deletions
  1. 1 0
      Gemfile
  2. 3 0
      Gemfile.lock
  3. 27 35
      app/models/translation.rb
  4. 90 6
      spec/models/translation_spec.rb

+ 1 - 0
Gemfile

@@ -5,6 +5,7 @@ ruby '2.4.4'
 gem 'rails', '5.1.5'
 
 # core - rails additions
+gem 'activerecord-import'
 gem 'activerecord-session_store'
 gem 'composite_primary_keys'
 gem 'json'

+ 3 - 0
Gemfile.lock

@@ -49,6 +49,8 @@ GEM
       activemodel (= 5.1.5)
       activesupport (= 5.1.5)
       arel (~> 8.0)
+    activerecord-import (0.25.0)
+      activerecord (>= 3.2)
     activerecord-nulldb-adapter (0.3.7)
       activerecord (>= 2.0.0)
     activerecord-session_store (1.1.0)
@@ -486,6 +488,7 @@ PLATFORMS
   ruby
 
 DEPENDENCIES
+  activerecord-import
   activerecord-nulldb-adapter
   activerecord-session_store
   argon2

+ 27 - 35
app/models/translation.rb

@@ -342,46 +342,38 @@ Get source file at https://i18n.zammad.com/api/v1/translations_empty_translation
     true
   end
 
+  def self.remote_translation_need_update?(raw, translations)
+    translations.each do |row|
+      next if row[1] != raw['locale']
+      next if row[2] != raw['source']
+      next if row[3] != raw['format']
+      return false if row[4] == raw['target'] # no update if target is still the same
+      return false if row[4] != row[5] # no update if translation has already changed
+      return [true, Translation.find(row[0])]
+    end
+    [true, nil]
+  end
+
   private_class_method def self.to_database(locale, data)
-    translations = Translation.where(locale: locale).all
+    translations = Translation.where(locale: locale).pluck(:id, :locale, :source, :format, :target, :target_initial).to_a
     ActiveRecord::Base.transaction do
+      translations_to_import = []
       data.each do |translation_raw|
-
-        # handle case insensitive sql
-        translation = nil
-        translations.each do |item|
-          next if item.format != translation_raw['format']
-          next if item.source != translation_raw['source']
-          translation = item
-          break
-        end
-        if translation
-
-          # verify if update is needed
-          update_needed = false
-          translation_raw.each_key do |key|
-
-            # if translation target has changes
-            next if translation_raw[key] == translation.target
-
-            # do not update translations which are already changed by user
-            if translation.target == translation.target_initial
-              update_needed = true
-              break
-            end
-          end
-          if update_needed
-            translation.update!(translation_raw.symbolize_keys!)
-            translation.save
-          end
+        result = Translation.remote_translation_need_update?(translation_raw, translations)
+        next if result == false
+        next if result.class != Array
+        if result[1]
+          result[1].update!(translation_raw.symbolize_keys!)
+          result[1].save
         else
-          if !UserInfo.current_user_id
-            translation_raw['updated_by_id'] = 1
-            translation_raw['created_by_id'] = 1
-          end
-          Translation.create(translation_raw.symbolize_keys!)
+          translation_raw['updated_by_id'] = UserInfo.current_user_id || 1
+          translation_raw['created_by_id'] = UserInfo.current_user_id || 1
+          translations_to_import.push Translation.new(translation_raw.symbolize_keys!)
         end
       end
+      if translations_to_import.present?
+        Translation.import translations_to_import
+      end
     end
   end
 
@@ -401,7 +393,7 @@ Get source file at https://i18n.zammad.com/api/v1/translations_empty_translation
   private
 
   def set_initial
-    return true if target_initial
+    return true if target_initial.present?
     return true if target_initial == ''
     self.target_initial = target
     true

+ 90 - 6
spec/models/translation_spec.rb

@@ -2,9 +2,10 @@ require 'rails_helper'
 
 RSpec.describe Translation do
 
+  Translation.where(locale: 'de-de').destroy_all
+  Translation.sync('de-de')
+
   context 'default translations' do
-    Translation.reset('de-de')
-    Translation.sync('de-de')
 
     it 'en with existing word' do
       expect(Translation.translate('en', 'New')).to eq('New')
@@ -28,13 +29,82 @@ RSpec.describe Translation do
 
   end
 
-  context 'custom translation tests' do
-    Translation.where(locale: 'de-de').destroy_all
-    Translation.sync('de-de')
+  context 'remote_translation_need_update? tests' do
+
+    it 'translation is still the same' do
+      translation = Translation.where(locale: 'de-de', format: 'string').last
+      translations = Translation.where(locale: 'de-de').pluck(:id, :locale, :source, :format, :target, :target_initial).to_a
+      expect(
+        Translation.remote_translation_need_update?(
+          {
+            'source' => translation.source,
+            'format' => translation.format,
+            'locale' => translation.locale,
+            'target' => translation.target,
+            'target_initial' => translation.target_initial,
+          }, translations
+        )
+      ).to be false
+    end
+
+    it 'translation target has locally changed' do
+      translation = Translation.where(locale: 'de-de', format: 'string').last
+      translation.target = 'some new translation'
+      translation.save!
+      translations = Translation.where(locale: 'de-de').pluck(:id, :locale, :source, :format, :target, :target_initial).to_a
+      expect(
+        Translation.remote_translation_need_update?(
+          {
+            'source' => translation.source,
+            'format' => translation.format,
+            'locale' => translation.locale,
+            'target' => translation.target,
+            'target_initial' => translation.target_initial,
+          }, translations
+        )
+      ).to be false
+    end
+
+    it 'translation target has remotely changed' do
+      translation = Translation.where(locale: 'de-de', format: 'string').last
+      translations = Translation.where(locale: 'de-de').pluck(:id, :locale, :source, :format, :target, :target_initial).to_a
+      (result, translation_result) = Translation.remote_translation_need_update?(
+        {
+          'source' => translation.source,
+          'format' => translation.format,
+          'locale' => translation.locale,
+          'target' => 'some new translation by remote',
+          'target_initial' => 'some new translation by remote',
+        }, translations
+      )
+      expect(result).to be true
+      expect(translation_result.attributes).to eq translation.attributes
+    end
+
+    it 'translation target has remotely and locally changed' do
+      translation = Translation.where(locale: 'de-de', format: 'string').last
+      translation.target = 'some new translation'
+      translation.save!
+      translations = Translation.where(locale: 'de-de').pluck(:id, :locale, :source, :format, :target, :target_initial).to_a
+      expect(
+        Translation.remote_translation_need_update?(
+          {
+            'source' => translation.source,
+            'format' => translation.format,
+            'locale' => translation.locale,
+            'target' => 'some new translation by remote',
+            'target_initial' => 'some new translation by remote',
+          }, translations
+        )
+      ).to be false
+    end
 
-    locale = 'de-de'
+  end
+
+  context 'custom translation tests' do
 
     it 'cycle of change and reload translation' do
+      locale = 'de-de'
 
       # check for non existing custom changes
       list = Translation.lang(locale)
@@ -123,4 +193,18 @@ RSpec.describe Translation do
 
   end
 
+  context 'sync duplicate tests' do
+
+    it 'check duplication of entries' do
+      Translation.where(locale: 'de-de').destroy_all
+      Translation.sync('de-de')
+      translation_count = Translation.where(locale: 'de-de').count
+      Translation.sync('de-de')
+      expect(
+        Translation.where(locale: 'de-de').count
+      ).to be translation_count
+    end
+
+  end
+
 end