From 3355364f371f5ef908f5f483439bd2edad43db2e Mon Sep 17 00:00:00 2001 From: Mark Allen <3417310+maallen@users.noreply.github.com> Date: Thu, 17 Aug 2023 09:19:37 +0100 Subject: [PATCH] Introduce 'delta-pull' option to the third party sync command (#10) Introduce a delta-pull option to the third party sync command, when enabled text unit imports will only occur for batches that contain updates. The implementation stores the checksum of a translated file pulled from a third party provider for a locale, on a subsequent sync compare the checksum of the current file with the stored checksum. If checksums match no changes have occurred and we will exit processing early for that locale. --- .../mojito/entity/ThirdPartyFileChecksum.java | 81 +++++ .../ThirdPartyFileChecksumRepository.java | 18 + .../thirdparty/ThirdPartyTMSSmartling.java | 83 +++-- .../ThirdPartyTMSSmartlingWithJson.java | 31 +- .../thirdparty/ThirdPartyTMSUtils.java | 58 ++++ .../smartling/SmartlingOptions.java | 16 +- .../service/tm/search/TextUnitSearcher.java | 4 + .../tm/search/TextUnitSearcherParameters.java | 10 + .../V64__third_party_sync_file_checksum.sql | 13 + .../ThirdPartyTMSSmartlingTest.java | 318 +++++++++++++++++- .../ThirdPartyTMSSmartlingWithJsonTest.java | 22 +- 11 files changed, 607 insertions(+), 47 deletions(-) create mode 100644 webapp/src/main/java/com/box/l10n/mojito/entity/ThirdPartyFileChecksum.java create mode 100644 webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyFileChecksumRepository.java create mode 100644 webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSUtils.java create mode 100644 webapp/src/main/resources/db/migration/V64__third_party_sync_file_checksum.sql diff --git a/webapp/src/main/java/com/box/l10n/mojito/entity/ThirdPartyFileChecksum.java b/webapp/src/main/java/com/box/l10n/mojito/entity/ThirdPartyFileChecksum.java new file mode 100644 index 0000000000..577461dfba --- /dev/null +++ b/webapp/src/main/java/com/box/l10n/mojito/entity/ThirdPartyFileChecksum.java @@ -0,0 +1,81 @@ +package com.box.l10n.mojito.entity; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.ForeignKey; +import javax.persistence.Index; +import javax.persistence.JoinColumn; +import javax.persistence.ManyToOne; +import javax.persistence.Table; + +/** Entity that stores the checksum of a translated file downloaded via a third party sync. */ +@Entity +@Table( + name = "third_party_sync_file_checksum", + indexes = { + @Index( + name = "I__TPS_FILE_CHECKSUM__REPO_ID__LOCALE_ID__FILE_NAME", + columnList = "repository_id, locale_id, file_name", + unique = true), + }) +public class ThirdPartyFileChecksum extends AuditableEntity { + + @ManyToOne(optional = false) + @JoinColumn( + name = "repository_id", + foreignKey = @ForeignKey(name = "FK__TPS_FILE_CHECKSUM__REPO__ID")) + private Repository repository; + + @Column(name = "file_name") + private String fileName; + + @ManyToOne(optional = false) + @JoinColumn( + name = "locale_id", + foreignKey = @ForeignKey(name = "FK__TPS_FILE_CHECKSUM__LOCALE__ID")) + private Locale locale; + + @Column(name = "md5") + private String md5; + + public ThirdPartyFileChecksum() {} + + public ThirdPartyFileChecksum(Repository repository, String fileName, Locale locale, String md5) { + this.repository = repository; + this.fileName = fileName; + this.locale = locale; + this.md5 = md5; + } + + public String getMd5() { + return md5; + } + + public void setMd5(String checksum) { + this.md5 = checksum; + } + + public Locale getLocale() { + return locale; + } + + public Repository getRepository() { + return repository; + } + + public String getFileName() { + return fileName; + } + + public void setLocale(Locale locale) { + this.locale = locale; + } + + public void setRepository(Repository repository) { + this.repository = repository; + } + + public void setFileName(String fileName) { + this.fileName = fileName; + } +} diff --git a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyFileChecksumRepository.java b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyFileChecksumRepository.java new file mode 100644 index 0000000000..2cb604e672 --- /dev/null +++ b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyFileChecksumRepository.java @@ -0,0 +1,18 @@ +package com.box.l10n.mojito.service.thirdparty; + +import com.box.l10n.mojito.entity.Locale; +import com.box.l10n.mojito.entity.Repository; +import com.box.l10n.mojito.entity.ThirdPartyFileChecksum; +import java.util.Optional; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.rest.core.annotation.RepositoryRestResource; + +@RepositoryRestResource(exported = false) +public interface ThirdPartyFileChecksumRepository + extends JpaRepository { + + Optional findById(Long thirdPartyFileChecksumId); + + Optional findByRepositoryAndFileNameAndLocale( + Repository repository, String fileName, Locale locale); +} diff --git a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartling.java b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartling.java index dda767b8c8..55ff818fcf 100644 --- a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartling.java +++ b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartling.java @@ -1,6 +1,7 @@ package com.box.l10n.mojito.service.thirdparty; import static com.box.l10n.mojito.android.strings.AndroidPluralQuantity.MANY; +import static com.box.l10n.mojito.service.thirdparty.ThirdPartyTMSUtils.isFileEqualToPreviousRun; import static com.box.l10n.mojito.service.thirdparty.smartling.SmartlingFileUtils.getOutputSourceFile; import static com.box.l10n.mojito.service.thirdparty.smartling.SmartlingFileUtils.getOutputTargetFile; import static com.box.l10n.mojito.service.thirdparty.smartling.SmartlingFileUtils.isPluralFile; @@ -98,6 +99,8 @@ public class ThirdPartyTMSSmartling implements ThirdPartyTMS { private final MeterRegistry meterRegistry; + private final ThirdPartyFileChecksumRepository thirdPartyFileChecksumRepository; + private final Set supportedImageExtensions = Sets.newHashSet("png", "jpg", "jpeg", "gif", "tiff"); @@ -115,7 +118,8 @@ public ThirdPartyTMSSmartling( ThirdPartyTMSSmartlingWithJson thirdPartyTMSSmartlingWithJson, ThirdPartyTMSSmartlingGlossary thirdPartyTMSSmartlingGlossary, AssetTextUnitToTMTextUnitRepository assetTextUnitToTMTextUnitRepository, - MeterRegistry meterRegistry) { + MeterRegistry meterRegistry, + ThirdPartyFileChecksumRepository thirdPartyFileChecksumRepository) { this( smartlingClient, textUnitSearcher, @@ -126,7 +130,8 @@ public ThirdPartyTMSSmartling( thirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, DEFAULT_BATCH_SIZE, - meterRegistry); + meterRegistry, + thirdPartyFileChecksumRepository); } public ThirdPartyTMSSmartling( @@ -139,7 +144,8 @@ public ThirdPartyTMSSmartling( ThirdPartyTMSSmartlingGlossary thirdPartyTMSSmartlingGlossary, AssetTextUnitToTMTextUnitRepository assetTextUnitToTMTextUnitRepository, int batchSize, - MeterRegistry meterRegistry) { + MeterRegistry meterRegistry, + ThirdPartyFileChecksumRepository thirdPartyFileChecksumRepository) { this.smartlingClient = smartlingClient; this.assetPathAndTextUnitNameKeys = assetPathAndTextUnitNameKeys; this.textUnitBatchImporterService = textUnitBatchImporterService; @@ -150,6 +156,7 @@ public ThirdPartyTMSSmartling( this.thirdPartyTMSSmartlingGlossary = thirdPartyTMSSmartlingGlossary; this.assetTextUnitToTMTextUnitRepository = assetTextUnitToTMTextUnitRepository; this.meterRegistry = meterRegistry; + this.thirdPartyFileChecksumRepository = thirdPartyFileChecksumRepository; } @Override @@ -529,14 +536,21 @@ public void pull( String skipAssetsWithPathPattern, List optionList) { + SmartlingOptions options = SmartlingOptions.parseList(optionList); + meterRegistry - .timer("SmartlingSync.pull", Tags.of("repository", repository.getName())) + .timer( + "SmartlingSync.pull", + Tags.of( + "repository", + repository.getName(), + "deltaPull", + Boolean.toString(options.isDeltaPull()))) .record( () -> { - SmartlingOptions options = SmartlingOptions.parseList(optionList); - if (options.isJsonSync()) { - thirdPartyTMSSmartlingWithJson.pull(repository, projectId, localeMapping); + thirdPartyTMSSmartlingWithJson.pull( + repository, projectId, localeMapping, options.isDeltaPull()); return; } @@ -602,7 +616,12 @@ private void processPullBatch( .timer( "SmartlingSync.processPullBatch", Tags.of( - "repository", repository.getName(), "locale", locale.getLocale().getBcp47Tag())) + "repository", + repository.getName(), + "locale", + locale.getLocale().getBcp47Tag(), + "deltaPull", + Boolean.toString(options.isDeltaPull()))) .record( () -> { String localeTag = locale.getLocale().getBcp47Tag(); @@ -648,6 +667,23 @@ private void processPullBatch( new SmartlingClientException( "Error with download from Smartling, file content string is not present.")); + if (options.isDeltaPull() + && isFileEqualToPreviousRun( + thirdPartyFileChecksumRepository, + repository, + locale.getLocale(), + fileName, + fileContent, + meterRegistry)) { + logger.info( + "Checksum match for " + + fileName + + " in locale " + + localeTag + + ", skipping text unit import."); + return; + } + List textUnits; try { @@ -893,7 +929,7 @@ private Stream> partitionSingulars( String localeTag, String skipTextUnitsWithPattern, String skipAssetsWithPathPattern) { - return partitionedStream( + TextUnitSearcherParameters parameters = baseParams( repositoryId, localeTag, @@ -901,8 +937,9 @@ private Stream> partitionSingulars( skipAssetsWithPathPattern, true, true, - null), - textUnitSearcher::search); + null); + parameters.setOrderByTextUnitID(true); + return partitionedStream(parameters, textUnitSearcher::search); } private Stream> partitionSingulars( @@ -911,7 +948,7 @@ private Stream> partitionSingulars( String skipTextUnitsWithPattern, String skipAssetsWithPathPattern, String includeTextUnitWithPattern) { - return partitionedStream( + TextUnitSearcherParameters parameters = baseParams( repositoryId, localeTag, @@ -920,8 +957,9 @@ private Stream> partitionSingulars( true, true, null, - includeTextUnitWithPattern), - textUnitSearcher::search); + includeTextUnitWithPattern); + parameters.setOrderByTextUnitID(true); + return partitionedStream(parameters, textUnitSearcher::search); } private Stream> partitionPlurals( @@ -929,7 +967,7 @@ private Stream> partitionPlurals( String localeTag, String skipTextUnitsWithPattern, String skipAssetsWithPathPattern) { - return partitionedStream( + TextUnitSearcherParameters parameters = baseParams( repositoryId, localeTag, @@ -937,8 +975,9 @@ private Stream> partitionPlurals( skipAssetsWithPathPattern, false, false, - "%"), - textUnitSearcher::search); + "%"); + parameters.setOrderByTextUnitID(true); + return partitionedStream(parameters, textUnitSearcher::search); } private Stream> partitionPlurals( @@ -961,7 +1000,7 @@ private Stream> partitionPlurals( .collect(Collectors.toList())); } - return partitionedStream( + TextUnitSearcherParameters parameters = baseParams( repositoryId, localeTag, @@ -970,8 +1009,11 @@ private Stream> partitionPlurals( false, false, "%", - includeTextUnitsWithPattern), - searchFunction); + includeTextUnitsWithPattern); + + parameters.setOrderByTextUnitID(true); + + return partitionedStream(parameters, searchFunction); } private Stream> partitionedStream( @@ -1037,7 +1079,6 @@ private TextUnitSearcherParameters baseParams( result.setPluralFormsExcluded(pluralFormsExcluded); result.setSkipTextUnitWithPattern(skipTextUnitsWithPattern); result.setSkipAssetPathWithPattern(skipAssetsWithPathPattern); - if (!Strings.isNullOrEmpty(pluralFormOther)) { result.setPluralFormOther(pluralFormOther); } diff --git a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartlingWithJson.java b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartlingWithJson.java index 9c10a5ff37..63b947ed62 100644 --- a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartlingWithJson.java +++ b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartlingWithJson.java @@ -1,5 +1,6 @@ package com.box.l10n.mojito.service.thirdparty; +import static com.box.l10n.mojito.service.thirdparty.ThirdPartyTMSUtils.isFileEqualToPreviousRun; import static com.box.l10n.mojito.service.thirdparty.smartling.SmartlingFileUtils.isPluralFile; import com.box.l10n.mojito.entity.Repository; @@ -58,6 +59,8 @@ public class ThirdPartyTMSSmartlingWithJson { MeterRegistry meterRegistry; + ThirdPartyFileChecksumRepository thirdPartyFileChecksumRepository; + int batchSize = 5000; public ThirdPartyTMSSmartlingWithJson( @@ -66,13 +69,15 @@ public ThirdPartyTMSSmartlingWithJson( TextUnitSearcher textUnitSearcher, TextUnitBatchImporterService textUnitBatchImporterService, SmartlingJsonKeys smartlingJsonKeys, - MeterRegistry meterRegistry) { + MeterRegistry meterRegistry, + ThirdPartyFileChecksumRepository thirdPartyFileChecksumRepository) { this.smartlingClient = smartlingClient; this.smartlingJsonConverter = smartlingJsonConverter; this.textUnitSearcher = textUnitSearcher; this.textUnitBatchImporterService = textUnitBatchImporterService; this.smartlingJsonKeys = smartlingJsonKeys; this.meterRegistry = meterRegistry; + this.thirdPartyFileChecksumRepository = thirdPartyFileChecksumRepository; } void push( @@ -187,7 +192,11 @@ void removeFileForBatchNumberGreaterOrEquals( .block()); } - void pull(Repository repository, String projectId, Map localeMapping) { + void pull( + Repository repository, + String projectId, + Map localeMapping, + boolean isDeltaPull) { List repositoryFilesFromProject = getRepositoryFilesFromProject(repository, projectId); @@ -201,6 +210,23 @@ void pull(Repository repository, String projectId, Map localeMap String localizedFileContent = getLocalizedFileContent(projectId, file, smartlingLocale, false); + if (isDeltaPull + && isFileEqualToPreviousRun( + thirdPartyFileChecksumRepository, + repository, + repositoryLocale.getLocale(), + file.getFileUri(), + localizedFileContent, + meterRegistry)) { + logger.info( + "Checksum match for " + + file.getFileUri() + + " in locale " + + repositoryLocale.getLocale().getBcp47Tag() + + ", skipping text unit import."); + return; + } + ImmutableList textUnitDTOS = smartlingJsonConverter.jsonStringToTextUnitDTOs( localizedFileContent, TextUnitDTO::setTarget); @@ -339,6 +365,7 @@ PageFetcherOffsetAndLimitSplitIterator getSourceTextUnitIterator( parameters.setOffset(offset); parameters.setLimit(limit); parameters.setPluralFormsFiltered(true); + parameters.setOrderByTextUnitID(true); List search = textUnitSearcher.search(parameters); return search; }, diff --git a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSUtils.java b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSUtils.java new file mode 100644 index 0000000000..1ee41ffc91 --- /dev/null +++ b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSUtils.java @@ -0,0 +1,58 @@ +package com.box.l10n.mojito.service.thirdparty; + +import com.box.l10n.mojito.entity.Locale; +import com.box.l10n.mojito.entity.Repository; +import com.box.l10n.mojito.entity.ThirdPartyFileChecksum; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Tags; +import java.util.Optional; +import org.apache.commons.codec.digest.DigestUtils; +import org.joda.time.DateTime; + +public class ThirdPartyTMSUtils { + + public static boolean isFileEqualToPreviousRun( + ThirdPartyFileChecksumRepository thirdPartyFileChecksumRepository, + Repository repository, + Locale locale, + String fileName, + String fileContent, + MeterRegistry meterRegistry) { + + boolean isChecksumEqual = false; + + String currentChecksum = DigestUtils.md5Hex(fileContent); + + Optional thirdPartyFileChecksumOpt = + thirdPartyFileChecksumRepository.findByRepositoryAndFileNameAndLocale( + repository, fileName, locale); + if (thirdPartyFileChecksumOpt.isPresent() + && thirdPartyFileChecksumOpt.get().getMd5().equals(currentChecksum)) { + isChecksumEqual = true; + } else if (thirdPartyFileChecksumOpt.isPresent()) { + ThirdPartyFileChecksum thirdPartyFileChecksum = thirdPartyFileChecksumOpt.get(); + thirdPartyFileChecksum.setMd5(currentChecksum); + thirdPartyFileChecksum.setLastModifiedDate(new DateTime()); + thirdPartyFileChecksumRepository.save(thirdPartyFileChecksum); + } else { + thirdPartyFileChecksumRepository.save( + new ThirdPartyFileChecksum(repository, fileName, locale, currentChecksum)); + } + + meterRegistry + .counter( + "ThirdPartyTMSUtils.deltaPullFilesProcessed", + Tags.of( + "repository", + repository.getName(), + "locale", + locale.getBcp47Tag(), + "fileName", + fileName, + "shortCircuited", + Boolean.toString(isChecksumEqual))) + .increment(); + + return isChecksumEqual; + } +} diff --git a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/smartling/SmartlingOptions.java b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/smartling/SmartlingOptions.java index 4de5a3858a..9441ae16fc 100644 --- a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/smartling/SmartlingOptions.java +++ b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/smartling/SmartlingOptions.java @@ -22,6 +22,8 @@ public final class SmartlingOptions { public static final String GLOSSARY_SYNC = "glossary-sync"; public static final String PUSH_TRANSALTION_BRANCH_NAME = "push-translation-branch-name"; + public static final String DELTA_PULL = "delta-pull"; + private final Set pluralFixForLocales; private final String placeholderFormat; private final String customPlaceholderFormat; @@ -32,6 +34,8 @@ public final class SmartlingOptions { private final boolean isGlossarySync; private final String pushTranslationBranchName; + private final boolean isDeltaPull; + public SmartlingOptions( Set pluralFixForLocales, String placeholderFormat, @@ -41,7 +45,8 @@ public SmartlingOptions( String requestId, boolean isJsonSync, boolean isGlossarySync, - String pushTranslationBranchName) { + String pushTranslationBranchName, + boolean isDeltaPull) { this.pluralFixForLocales = pluralFixForLocales; this.placeholderFormat = placeholderFormat; this.customPlaceholderFormat = customPlaceholderFormat; @@ -51,6 +56,7 @@ public SmartlingOptions( this.isJsonSync = isJsonSync; this.isGlossarySync = isGlossarySync; this.pushTranslationBranchName = pushTranslationBranchName; + this.isDeltaPull = isDeltaPull; } public static SmartlingOptions parseList(List options) { @@ -70,6 +76,7 @@ public static SmartlingOptions parseList(List options) { String isJsonSync = map.get(JSON_SYNC); String isGlossarySync = map.get(GLOSSARY_SYNC); String pushTranslationBranchName = map.get(PUSH_TRANSALTION_BRANCH_NAME); + String deltaSync = map.get(DELTA_PULL); return new SmartlingOptions( pluralFixStr.isEmpty() @@ -82,7 +89,8 @@ public static SmartlingOptions parseList(List options) { requestId, parseBoolean(isJsonSync), parseBoolean(isGlossarySync), - pushTranslationBranchName); + pushTranslationBranchName, + parseBoolean(deltaSync)); } public Set getPluralFixForLocales() { @@ -120,4 +128,8 @@ public boolean isGlossarySync() { public String getPushTranslationBranchName() { return pushTranslationBranchName; } + + public boolean isDeltaPull() { + return isDeltaPull; + } } diff --git a/webapp/src/main/java/com/box/l10n/mojito/service/tm/search/TextUnitSearcher.java b/webapp/src/main/java/com/box/l10n/mojito/service/tm/search/TextUnitSearcher.java index bbb5906f9a..e5e737debe 100644 --- a/webapp/src/main/java/com/box/l10n/mojito/service/tm/search/TextUnitSearcher.java +++ b/webapp/src/main/java/com/box/l10n/mojito/service/tm/search/TextUnitSearcher.java @@ -441,6 +441,10 @@ NativeCriteria getCriteriaForSearch(TextUnitSearcherParameters searchParameters) c.setOffset(searchParameters.getOffset()); } + if (searchParameters.isOrderedByTextUnitID()) { + c.setOrder(NativeExps.order().add("tu.id", NativeOrderExp.OrderType.ASC)); + } + if (searchParameters instanceof TextUnitSearcherParametersForTesting) { TextUnitSearcherParametersForTesting textUnitSearcherParametersForTesting = (TextUnitSearcherParametersForTesting) searchParameters; diff --git a/webapp/src/main/java/com/box/l10n/mojito/service/tm/search/TextUnitSearcherParameters.java b/webapp/src/main/java/com/box/l10n/mojito/service/tm/search/TextUnitSearcherParameters.java index 4d7d5856be..7a3a8644e9 100644 --- a/webapp/src/main/java/com/box/l10n/mojito/service/tm/search/TextUnitSearcherParameters.java +++ b/webapp/src/main/java/com/box/l10n/mojito/service/tm/search/TextUnitSearcherParameters.java @@ -39,6 +39,8 @@ public class TextUnitSearcherParameters { boolean untranslatedOrTranslationNeeded = false; boolean pluralFormsFiltered = true; boolean pluralFormsExcluded = false; + + boolean isOrderedByTextUnitID = false; Long pluralFormId; Boolean doNotTranslateFilter; DateTime tmTextUnitCreatedBefore; @@ -308,4 +310,12 @@ public String getSkipAssetPathWithPattern() { public void setSkipAssetPathWithPattern(String skipAssetPathWithPattern) { this.skipAssetPathWithPattern = skipAssetPathWithPattern; } + + public boolean isOrderedByTextUnitID() { + return isOrderedByTextUnitID; + } + + public void setOrderByTextUnitID(boolean ordered) { + isOrderedByTextUnitID = ordered; + } } diff --git a/webapp/src/main/resources/db/migration/V64__third_party_sync_file_checksum.sql b/webapp/src/main/resources/db/migration/V64__third_party_sync_file_checksum.sql new file mode 100644 index 0000000000..57e3d4e674 --- /dev/null +++ b/webapp/src/main/resources/db/migration/V64__third_party_sync_file_checksum.sql @@ -0,0 +1,13 @@ +create table third_party_sync_file_checksum ( + id bigint(20) NOT NULL AUTO_INCREMENT, + repository_id bigint not null, + locale_id bigint not null, + md5 char(32) not null, + file_name varchar(255) not null, + created_date datetime not null, + last_modified_date datetime, + primary key (id) +); +alter table third_party_sync_file_checksum add constraint FK__TPS_FILE_CHECKSUM__REPO__ID foreign key (repository_id) references repository (id); +alter table third_party_sync_file_checksum add constraint FK__TPS_FILE_CHECKSUM__LOCALE__ID foreign key (locale_id) references locale (id); +create unique index I__TPS_FILE_CHECKSUM__REPO_ID__LOCALE_ID__FILE_NAME on third_party_sync_file_checksum(repository_id, locale_id, file_name); \ No newline at end of file diff --git a/webapp/src/test/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartlingTest.java b/webapp/src/test/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartlingTest.java index 1829cdec5f..825c07449e 100644 --- a/webapp/src/test/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartlingTest.java +++ b/webapp/src/test/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartlingTest.java @@ -134,6 +134,8 @@ public class ThirdPartyTMSSmartlingTest extends ServiceTestBase { @Autowired MeterRegistry meterRegistry; + @Autowired ThirdPartyFileChecksumRepository mockThirdPartyFileChecksumRepository; + StubSmartlingResultProcessor resultProcessor; @Mock TextUnitBatchImporterService mockTextUnitBatchImporterService; @@ -184,7 +186,8 @@ public void setUp() { mockThirdPartyTMSSmartlingWithJson, mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); mapper = new AndroidStringDocumentMapper(pluralSep, null); RetryBackoffSpec retryConfiguration = @@ -235,7 +238,8 @@ public void testPushInBatchesWithSingularsAndNoPlurals() mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, batchSize, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); TM tm = repository.getTm(); Asset asset = @@ -288,7 +292,8 @@ public void testRetryDuringPush() throws RepositoryNameAlreadyUsedException { mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, batchSize, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); // throw timeout exception for first request, following request should be successful when(smartlingClient.uploadFile(any(), any(), any(), any(), any(), any(), any())) .thenThrow( @@ -353,7 +358,8 @@ public void testRetriesExhaustedDuringPush() throws RepositoryNameAlreadyUsedExc mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, batchSize, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); TM tm = repository.getTm(); Asset asset = @@ -403,7 +409,8 @@ public void testPushInBatchesWithNoSingularsAndPlurals() mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, batchSize, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); TM tm = repository.getTm(); Asset asset = @@ -461,7 +468,8 @@ public void testPushInBatchesWithSingularsAndPlurals() throws RepositoryNameAlre mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, batchSize, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); TM tm = repository.getTm(); Asset asset = @@ -675,7 +683,8 @@ public void testPullNoBatches() throws RepositoryLocaleCreationException { mockThirdPartyTMSSmartlingWithJson, mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); tmsSmartling.pull( repository, "projectId", pluralSep, localeMapping, null, null, Collections.emptyList()); @@ -789,7 +798,8 @@ public void testRetrySuccessDuringPull() mockThirdPartyTMSSmartlingWithJson, mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); tmsSmartling.pull( repository, "projectId", pluralSep, localeMapping, null, null, Collections.emptyList()); @@ -883,7 +893,8 @@ public void testRetriesExhaustedDuringPull() mockThirdPartyTMSSmartlingWithJson, mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); RuntimeException exception = assertThrows( SmartlingClientException.class, @@ -943,7 +954,8 @@ public void testPullNoBatchesLocaleMapping() throws RepositoryLocaleCreationExce mockThirdPartyTMSSmartlingWithJson, mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); tmsSmartling.pull( repository, "projectId", pluralSep, localeMapping, null, null, Collections.emptyList()); @@ -1052,7 +1064,8 @@ public void testPullNoBatchesPluralFix() throws RepositoryLocaleCreationExceptio mockThirdPartyTMSSmartlingWithJson, mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); tmsSmartling.pull( repository, "projectId", @@ -1185,7 +1198,8 @@ public void testPullDryRunNoBatches() throws RepositoryLocaleCreationException { mockThirdPartyTMSSmartlingWithJson, mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); tmsSmartling.pull( repository, "projectId", @@ -1199,6 +1213,271 @@ public void testPullDryRunNoBatches() throws RepositoryLocaleCreationException { .importTextUnits(textUnitListCaptor.capture(), eq(false), eq(true)); } + @Test + public void testPullShortCircuitIfNoChangesInTranslatedFiles() + throws RepositoryNameAlreadyUsedException, RepositoryLocaleCreationException { + + int batchSize = 3; + List tus; + List locales = new ArrayList<>(); + Map localeMapping = Collections.emptyMap(); + tmsSmartling = + new ThirdPartyTMSSmartling( + smartlingClient, + textUnitSearcher, + assetPathAndTextUnitNameKeys, + mockTextUnitBatchImporterService, + resultProcessor, + mockThirdPartyTMSSmartlingWithJson, + mockThirdPartyTMSSmartlingGlossary, + assetTextUnitToTMTextUnitRepository, + batchSize, + meterRegistry, + mockThirdPartyFileChecksumRepository); + Repository repository = + repositoryService.createRepository(testIdWatcher.getEntityName("batchRepo")); + Locale frCA = localeService.findByBcp47Tag("fr-CA"); + Locale jaJP = localeService.findByBcp47Tag("ja-JP"); + repositoryService.addRepositoryLocale(repository, frCA.getBcp47Tag()); + repositoryService.addRepositoryLocale(repository, jaJP.getBcp47Tag()); + locales.add(frCA); + locales.add(jaJP); + + TM tm = repository.getTm(); + Asset asset = + assetService.createAssetWithContent(repository.getId(), "fake_for_test", "fake for test"); + AssetExtraction assetExtraction = new AssetExtraction(); + assetExtraction.setAsset(asset); + assetExtraction = assetExtractionRepository.save(assetExtraction); + + PluralForm one = pluralFormService.findByPluralFormString("one"); + + List singularIds = new ArrayList<>(); + int singularTextUnits = 14; + int singularBatches = 5; + + for (int i = 0; i < singularTextUnits; i++) { + String name = "singular_message_test" + i; + String content = "Singular Message Test #" + i; + String comment = "Singular Comment" + i; + TMTextUnit textUnit = + tmService.addTMTextUnit(tm.getId(), asset.getId(), name, content, comment); + singularIds.add(textUnit.getId()); + assetExtractionService.createAssetTextUnit(assetExtraction, name, content, comment); + } + + List pluralIds = new ArrayList<>(); + int pluralTextUnits = 8; + int pluralBatches = 3; + + for (int i = 0; i < pluralTextUnits; i++) { + String name = "plural_message" + i + "_one"; + String content = "Plural Message Test #" + i; + String comment = "Plural Comment" + i; + String pluralFormOther = "plural_form_other" + i; + + TMTextUnit textUnit = + tmService.addTMTextUnit(tm, asset, name, content, comment, null, one, pluralFormOther); + pluralIds.add(textUnit.getId()); + assetExtractionService.createAssetTextUnit(assetExtraction, name, content, comment); + } + + prepareAssetAndTextUnits(assetExtraction, asset, tm); + + tus = searchTextUnits(singularIds); + Iterable> singularIt = Iterables.partition(tus, batchSize); + int i = 0; + for (List textUnits : singularIt) { + for (Locale l : locales) { + doReturn(localizedContent(textUnits, l)) + .when(smartlingClient) + .downloadPublishedFile( + eq("projectId"), + eq(l.getBcp47Tag()), + eq(singularFileName(repository, i)), + eq(false)); + } + i++; + } + + tus = searchTextUnits(pluralIds); + Iterable> pluralIt = Iterables.partition(tus, batchSize); + i = 0; + for (List textUnits : pluralIt) { + for (Locale l : locales) { + doReturn(localizedContent(textUnits, l)) + .when(smartlingClient) + .downloadPublishedFile( + eq("projectId"), eq(l.getBcp47Tag()), eq(pluralFileName(repository, i)), eq(false)); + } + i++; + } + + // Execute an initial pull that will record translated files checksums and verify text units are + // imported + List options = new ArrayList<>(); + options.add("delta-pull=true"); + tmsSmartling.pull(repository, "projectId", pluralSep, localeMapping, null, null, options); + + verify(mockTextUnitBatchImporterService, atLeastOnce()) + .importTextUnits(textUnitListCaptor.capture(), eq(false), eq(true)); + + Mockito.clearInvocations(mockTextUnitBatchImporterService); + + // Now run the same pull again, text unit imports should be skipped as file checksums from + // previous run exist and match. + tmsSmartling.pull(repository, "projectId", pluralSep, localeMapping, null, null, options); + + verify(mockTextUnitBatchImporterService, times(0)) + .importTextUnits(textUnitListCaptor.capture(), eq(false), eq(true)); + } + + @Test + public void testPullShortCircuitIfChangesInTranslatedFiles() + throws RepositoryNameAlreadyUsedException, RepositoryLocaleCreationException { + + int batchSize = 3; + List tus; + List locales = new ArrayList<>(); + Map localeMapping = Collections.emptyMap(); + tmsSmartling = + new ThirdPartyTMSSmartling( + smartlingClient, + textUnitSearcher, + assetPathAndTextUnitNameKeys, + mockTextUnitBatchImporterService, + resultProcessor, + mockThirdPartyTMSSmartlingWithJson, + mockThirdPartyTMSSmartlingGlossary, + assetTextUnitToTMTextUnitRepository, + batchSize, + meterRegistry, + mockThirdPartyFileChecksumRepository); + Repository repository = + repositoryService.createRepository(testIdWatcher.getEntityName("batchRepo")); + Locale frCA = localeService.findByBcp47Tag("fr-CA"); + Locale jaJP = localeService.findByBcp47Tag("ja-JP"); + repositoryService.addRepositoryLocale(repository, frCA.getBcp47Tag()); + repositoryService.addRepositoryLocale(repository, jaJP.getBcp47Tag()); + locales.add(frCA); + locales.add(jaJP); + + TM tm = repository.getTm(); + Asset asset = + assetService.createAssetWithContent(repository.getId(), "fake_for_test", "fake for test"); + AssetExtraction assetExtraction = new AssetExtraction(); + assetExtraction.setAsset(asset); + assetExtraction = assetExtractionRepository.save(assetExtraction); + + PluralForm one = pluralFormService.findByPluralFormString("one"); + + List singularIds = new ArrayList<>(); + int singularTextUnits = 14; + int singularBatches = 5; + + for (int i = 0; i < singularTextUnits; i++) { + String name = "singular_message_test" + i; + String content = "Singular Message Test #" + i; + String comment = "Singular Comment" + i; + TMTextUnit textUnit = + tmService.addTMTextUnit(tm.getId(), asset.getId(), name, content, comment); + singularIds.add(textUnit.getId()); + assetExtractionService.createAssetTextUnit(assetExtraction, name, content, comment); + } + + List pluralIds = new ArrayList<>(); + int pluralTextUnits = 8; + int pluralBatches = 3; + + for (int i = 0; i < pluralTextUnits; i++) { + String name = "plural_message" + i + "_one"; + String content = "Plural Message Test #" + i; + String comment = "Plural Comment" + i; + String pluralFormOther = "plural_form_other" + i; + + TMTextUnit textUnit = + tmService.addTMTextUnit(tm, asset, name, content, comment, null, one, pluralFormOther); + pluralIds.add(textUnit.getId()); + assetExtractionService.createAssetTextUnit(assetExtraction, name, content, comment); + } + + prepareAssetAndTextUnits(assetExtraction, asset, tm); + + tus = searchTextUnits(singularIds); + Iterable> singularIt = Iterables.partition(tus, batchSize); + int i = 0; + for (List textUnits : singularIt) { + for (Locale l : locales) { + doReturn(localizedContent(textUnits, l)) + .when(smartlingClient) + .downloadPublishedFile( + eq("projectId"), + eq(l.getBcp47Tag()), + eq(singularFileName(repository, i)), + eq(false)); + } + i++; + } + + tus = searchTextUnits(pluralIds); + Iterable> pluralIt = Iterables.partition(tus, batchSize); + i = 0; + for (List textUnits : pluralIt) { + for (Locale l : locales) { + doReturn(localizedContent(textUnits, l)) + .when(smartlingClient) + .downloadPublishedFile( + eq("projectId"), eq(l.getBcp47Tag()), eq(pluralFileName(repository, i)), eq(false)); + } + i++; + } + + // Execute an initial pull that will record translated files checksums and verify text units are + // imported + List options = new ArrayList<>(); + options.add("delta-pull=true"); + tmsSmartling.pull(repository, "projectId", pluralSep, localeMapping, null, null, options); + + verify(mockTextUnitBatchImporterService, atLeastOnce()) + .importTextUnits(textUnitListCaptor.capture(), eq(false), eq(true)); + + Mockito.clearInvocations(mockTextUnitBatchImporterService); + + i = 0; + for (List textUnits : singularIt) { + for (Locale l : locales) { + // add a new line to translated file to trigger checksum change + doReturn(localizedContent(textUnits, l) + "\n") + .when(smartlingClient) + .downloadPublishedFile( + eq("projectId"), + eq(l.getBcp47Tag()), + eq(singularFileName(repository, i)), + eq(false)); + } + i++; + } + + i = 0; + for (List textUnits : pluralIt) { + for (Locale l : locales) { + // add a new line to translated file to trigger checksum change + doReturn(localizedContent(textUnits, l) + "\n") + .when(smartlingClient) + .downloadPublishedFile( + eq("projectId"), eq(l.getBcp47Tag()), eq(pluralFileName(repository, i)), eq(false)); + } + i++; + } + + // Run the same pull again, text unit imports should occur as file checksums have changed since + // previous run + tmsSmartling.pull(repository, "projectId", pluralSep, localeMapping, null, null, options); + + verify(mockTextUnitBatchImporterService, atLeastOnce()) + .importTextUnits(textUnitListCaptor.capture(), eq(false), eq(true)); + } + @Test public void testPullInBatchesWithSingularsAndPlurals() throws RepositoryNameAlreadyUsedException, RepositoryLocaleCreationException { @@ -1218,7 +1497,8 @@ public void testPullInBatchesWithSingularsAndPlurals() mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, batchSize, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); Repository repository = repositoryService.createRepository(testIdWatcher.getEntityName("batchRepo")); Locale frCA = localeService.findByBcp47Tag("fr-CA"); @@ -1886,7 +2166,8 @@ public void testPushTranslationsInBatches() mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, batchSize, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); Repository repository = repositoryService.createRepository(testIdWatcher.getEntityName("batchRepo")); Locale frCA = localeService.findByBcp47Tag("fr-CA"); @@ -2039,7 +2320,8 @@ public void testBatchesFor() { mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, 3, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); assertThat(tmsSmartling.batchesFor(0)).isEqualTo(0); assertThat(tmsSmartling.batchesFor(1)).isEqualTo(1); @@ -2061,7 +2343,8 @@ public void testBatchesFor() { mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, 35, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); assertThat(tmsSmartling.batchesFor(0)).isEqualTo(0); assertThat(tmsSmartling.batchesFor(1)).isEqualTo(1); @@ -2081,7 +2364,8 @@ public void testBatchesFor() { mockThirdPartyTMSSmartlingGlossary, assetTextUnitToTMTextUnitRepository, 4231, - meterRegistry); + meterRegistry, + mockThirdPartyFileChecksumRepository); assertThat(tmsSmartling.batchesFor(0)).isEqualTo(0); assertThat(tmsSmartling.batchesFor(1)).isEqualTo(1); diff --git a/webapp/src/test/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartlingWithJsonTest.java b/webapp/src/test/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartlingWithJsonTest.java index c2da4e4850..636f45b2bb 100644 --- a/webapp/src/test/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartlingWithJsonTest.java +++ b/webapp/src/test/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSSmartlingWithJsonTest.java @@ -30,6 +30,7 @@ import java.util.Arrays; import java.util.Comparator; import java.util.List; +import java.util.Optional; import java.util.stream.Stream; import java.util.stream.StreamSupport; import org.junit.Assert; @@ -80,6 +81,8 @@ public class ThirdPartyTMSSmartlingWithJsonTest extends ServiceTestBase { @Autowired TextUnitSearcher textUnitSearcher; + @Mock ThirdPartyFileChecksumRepository thirdPartyFileChecksumRepositoryMock; + SmartlingJsonConverter smartlingJsonConverter = new SmartlingJsonConverter(ObjectMapper.withIndentedOutput(), new SmartlingJsonKeys()); @@ -173,7 +176,8 @@ public void testJsonWithICUMessagFormats() throws Exception { "eventually we should be able to pull", () -> { logger.debug("Pulling..."); - thirdPartyTMSSmartlingWithJson.pull(repository, testConfig.projectId, ImmutableMap.of()); + thirdPartyTMSSmartlingWithJson.pull( + repository, testConfig.projectId, ImmutableMap.of(), false); return true; }, 3, @@ -250,7 +254,8 @@ public void testGetTranslatedUnits() { ImmutableList.of(translatedTextUnitDto, untranslatedTextUnitDtoWithOriginalString); ThirdPartyTMSSmartlingWithJson thirdPartyTMSSmartlingWithJson = - new ThirdPartyTMSSmartlingWithJson(null, null, null, null, null, meterRegistryMock); + new ThirdPartyTMSSmartlingWithJson( + null, null, null, null, null, meterRegistryMock, thirdPartyFileChecksumRepositoryMock); ImmutableList result = thirdPartyTMSSmartlingWithJson.getTranslatedUnits( @@ -342,7 +347,7 @@ public void testPullWithUntranslatedUnits() throws Exception { Mockito.doCallRealMethod() .when(thirdPartyTMSSmartlingWithJsonMock) - .pull(repository, projectId, localeMapping); + .pull(repository, projectId, localeMapping, false); Mockito.when(thirdPartyTMSSmartlingWithJsonMock.hasEmptyTranslations(any())) .thenCallRealMethod(); Mockito.when(thirdPartyTMSSmartlingWithJsonMock.getTranslatedUnits(any(), any())) @@ -362,14 +367,21 @@ public void testPullWithUntranslatedUnits() throws Exception { thirdPartyTMSSmartlingWithJsonMock.smartlingJsonConverter = smartlingJsonConverter; thirdPartyTMSSmartlingWithJsonMock.textUnitBatchImporterService = textUnitBatchImporterServiceMock; + thirdPartyTMSSmartlingWithJsonMock.thirdPartyFileChecksumRepository = + thirdPartyFileChecksumRepositoryMock; Mockito.when( thirdPartyTMSSmartlingWithJsonMock.getLocalizedFileContent( projectId, smartlingFile, smartlingLocale, false)) .thenReturn(smartlingJsonResponseWithOriginalString); + Mockito.when( + thirdPartyFileChecksumRepositoryMock.findByRepositoryAndFileNameAndLocale( + isA(Repository.class), isA(String.class), isA(Locale.class))) + .thenReturn(Optional.empty()); + // For the first pass, mock a fully translated response - thirdPartyTMSSmartlingWithJsonMock.pull(repository, projectId, localeMapping); + thirdPartyTMSSmartlingWithJsonMock.pull(repository, projectId, localeMapping, false); ArgumentCaptor> dtoListCaptor = ArgumentCaptor.forClass(ImmutableList.class); @@ -393,7 +405,7 @@ public void testPullWithUntranslatedUnits() throws Exception { projectId, smartlingFile, smartlingLocale, true)) .thenReturn(smartlingJsonResponseWithOriginalString); - thirdPartyTMSSmartlingWithJsonMock.pull(repository, projectId, localeMapping); + thirdPartyTMSSmartlingWithJsonMock.pull(repository, projectId, localeMapping, false); dtoListCaptor = ArgumentCaptor.forClass(ImmutableList.class); Mockito.verify(textUnitBatchImporterServiceMock, times(2))