From 1cabf59156b696b713697148111bf868ae6c58ac Mon Sep 17 00:00:00 2001 From: Jean Aurambault Date: Wed, 19 Jun 2024 12:51:01 -0700 Subject: [PATCH] Phrase Connector support N Mojito repositories to 1 project mapping --- .../thirdparty/ThirdPartyTMSPhrase.java | 93 ++++++++++++++++--- .../thirdparty/phrase/PhraseClient.java | 86 +++++++++++++---- .../thirdparty/phrase/PhraseClientTest.java | 88 +++++++++++------- 3 files changed, 205 insertions(+), 62 deletions(-) diff --git a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSPhrase.java b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSPhrase.java index 400539493f..b511217dc8 100644 --- a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSPhrase.java +++ b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSPhrase.java @@ -31,6 +31,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; @@ -45,6 +46,7 @@ public class ThirdPartyTMSPhrase implements ThirdPartyTMS { static final String TAG_PREFIX = "push_"; + static final String TAG_PREFIX_WITH_REPOSITORY = "push_%s"; static Logger logger = LoggerFactory.getLogger(ThirdPartyTMSPhrase.class); @@ -56,6 +58,12 @@ public class ThirdPartyTMSPhrase implements ThirdPartyTMS { @Autowired RepositoryService repositoryService; + public ThirdPartyTMSPhrase() {} + + public ThirdPartyTMSPhrase(PhraseClient phraseClient) { + this.phraseClient = phraseClient; + } + @Override public void removeImage(String projectId, String imageId) { throw new UnsupportedOperationException("Remove image is not supported"); @@ -129,7 +137,7 @@ public void push( String text = getFileContent(pluralSeparator, search, true, null); - String tagForUpload = getTagForUpload(); + String tagForUpload = getTagForUpload(repository.getName()); phraseClient.uploadAndWait( projectId, repository.getSourceLocale().getBcp47Tag(), @@ -138,17 +146,62 @@ public void push( text, ImmutableList.of(tagForUpload)); - phraseClient.removeKeysNotTaggedWith(projectId, tagForUpload); + removeUnusedKeysAndTags(projectId, repository.getName(), tagForUpload); + } - List tagsToDelete = + /** + * Remove unused keys and tags + * + * + * + *

Explanation: + * + *

The N:1 relationship between Mojito and Phrase is maintained through this logic, allowing + * for efficient management of keys and tags. + */ + public void removeUnusedKeysAndTags( + String projectId, String repositoryName, String tagForUpload) { + + List tagsForOtherRepositories = phraseClient.listTags(projectId).stream() + .map(Tag::getName) + .filter(Objects::nonNull) + .filter(tagName -> tagName.startsWith(TAG_PREFIX)) .filter( - tag -> - tag.getName() != null - && !tag.getName().equals(tagForUpload) - && tag.getName().startsWith(TAG_PREFIX)) + tagName -> + !tagName.startsWith(TAG_PREFIX_WITH_REPOSITORY.formatted(repositoryName))) + .toList(); + + List allActiveTags = new ArrayList<>(tagsForOtherRepositories); + allActiveTags.add(tagForUpload); + + logger.info("All active tags: {}", allActiveTags); + phraseClient.removeKeysNotTaggedWith(projectId, allActiveTags); + + List pushTagsToDelete = + phraseClient.listTags(projectId).stream() + .map(Tag::getName) + .filter(Objects::nonNull) + .filter(tagName -> tagName.startsWith(TAG_PREFIX)) + .filter(tagName -> !allActiveTags.contains(tagName)) .toList(); - phraseClient.deleteTags(projectId, tagsToDelete); + + logger.info("Tags to delete: {}", pushTagsToDelete); + phraseClient.deleteTags(projectId, pushTagsToDelete); } private List getSourceTextUnitDTOs( @@ -185,12 +238,13 @@ private List getSourceTextUnitDTOsPluralOnly( return textUnitSearcher.search(parameters); } - public static String getTagForUpload() { + public static String getTagForUpload(String repositoryName) { ZonedDateTime zonedDateTime = JSR310Migration.dateTimeNowInUTC(); DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy_MM_dd_HH_mm_ss_SSS"); - return ("%s%s_%s") + return ("%s%s_%s_%s") .formatted( TAG_PREFIX, + repositoryName, formatter.format(zonedDateTime), Math.abs(UUID.randomUUID().getLeastSignificantBits() % 1000)); } @@ -210,10 +264,19 @@ public PollableFuture pull( Set repositoryLocalesWithoutRootLocale = repositoryService.getRepositoryLocalesWithoutRootLocale(repository); + String currentTags = getCurrentTagsForRepository(repository, projectId); + for (RepositoryLocale repositoryLocale : repositoryLocalesWithoutRootLocale) { String localeTag = repositoryLocale.getLocale().getBcp47Tag(); logger.info("Downloading locale: {} from Phrase", localeTag); - String fileContent = phraseClient.localeDownload(projectId, localeTag, "xml"); + + String fileContent = + phraseClient.localeDownload( + projectId, + localeTag, + "xml", + currentTags, + () -> getCurrentTagsForRepository(repository, projectId)); logger.info("file content from pull: {}", fileContent); @@ -230,6 +293,14 @@ public PollableFuture pull( return null; } + private String getCurrentTagsForRepository(Repository repository, String projectId) { + return phraseClient.listTags(projectId).stream() + .map(Tag::getName) + .filter(Objects::nonNull) + .filter(tagName -> tagName.startsWith(repository.getName())) + .collect(Collectors.joining(",")); + } + @Override public void pushTranslations( Repository repository, diff --git a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/phrase/PhraseClient.java b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/phrase/PhraseClient.java index 6acc3c824e..d5ffd75a27 100644 --- a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/phrase/PhraseClient.java +++ b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/phrase/PhraseClient.java @@ -24,6 +24,8 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import reactor.core.publisher.Mono; @@ -168,13 +170,35 @@ Upload uploadsApiUploadCreateWithRetry( .block(); } - public void removeKeysNotTaggedWith(String projectId, String tag) { - logger.info("Removing keys not tagged with: {}", tag); + /** + * Conducted tests on keysDeleteCollection using the -tags:tag1,tag2 option, and it + * operates as a filter for keys "not in any of the tags". + * + *

It removes keys that are not tagged with any of the provided tags. + * + *

For example: + * + *

+ * + *

Calling keysDeleteCollection with -tags:tag1,tag3 will remove + * kb. + */ + public void removeKeysNotTaggedWith(String projectId, List anyOfTheseTags) { + logger.info("Removing keys not tagged with any of the following tags: {}", anyOfTheseTags); Mono.fromCallable( () -> { KeysApi keysApi = new KeysApi(apiClient); - keysApi.keysDeleteCollection(projectId, null, null, "-tags:%s".formatted(tag), null); + keysApi.keysDeleteCollection( + projectId, + null, + null, + "-tags:%s".formatted(String.join(",", anyOfTheseTags)), + null); return null; }) .retryWhen( @@ -193,7 +217,18 @@ public void removeKeysNotTaggedWith(String projectId, String tag) { .block(); } - public String localeDownload(String projectId, String locale, String fileFormat) { + /** + * @param onTagErrorRefreshCallback with concurrent update, the tags could be updated during + * download. We don't want to retry the whole logic, so we provide a callback to refresh the + * tags + */ + public String localeDownload( + String projectId, + String locale, + String fileFormat, + String tags, + Supplier onTagErrorRefreshCallback) { + AtomicReference refTags = new AtomicReference<>(tags); return Mono.fromCallable( () -> { LocalesApi localesApi = new LocalesApi(apiClient); @@ -211,7 +246,7 @@ public String localeDownload(String projectId, String locale, String fileFormat) null, null, fileFormat, - null, + refTags.get(), null, null, null, @@ -234,11 +269,22 @@ public String localeDownload(String projectId, String locale, String fileFormat) }) .retryWhen( retryBackoffSpec.doBeforeRetry( - doBeforeRetry -> - logAttempt( - doBeforeRetry.failure(), - "Retrying failed attempt to localeDownload from Phrase, project id: %s, locale: %s" - .formatted(projectId, locale)))) + doBeforeRetry -> { + logAttempt( + doBeforeRetry.failure(), + "Retrying failed attempt to localeDownload from Phrase, project id: %s, locale: %s" + .formatted(projectId, locale)); + + if (getErrorMessageFromOptionalApiException(doBeforeRetry.failure()) + .contains("Invalid Download Options. Parameter tags ")) { + String newTags = onTagErrorRefreshCallback.get(); + logger.warn( + "Replacing old tags: {} with new tags: {} for download locale", + refTags.get(), + newTags); + refTags.set(newTags); + } + })) .doOnError( throwable -> rethrowExceptionWithLog( @@ -325,15 +371,17 @@ public List listTags(String projectId) { return tags; } - public void deleteTags(String projectId, List tags) { + public void deleteTags(String projectId, List tagNames) { + + logger.debug("Delete tags: {}", tagNames); + TagsApi tagsApi = new TagsApi(apiClient); Map exceptions = new LinkedHashMap<>(); - for (Tag tag : tags) { + for (String tagName : tagNames) { Mono.fromCallable( () -> { - logger.debug( - "Deleting tag: %s in project id: %s".formatted(tag.getName(), projectId)); - tagsApi.tagDelete(projectId, tag.getName(), null, null); + logger.debug("Deleting tag: %s in project id: %s".formatted(tagName, projectId)); + tagsApi.tagDelete(projectId, tagName, null, null); return null; }) .retryWhen( @@ -342,15 +390,15 @@ public void deleteTags(String projectId, List tags) { logAttempt( doBeforeRetry.failure(), "Retrying failed attempt to delete tag: %s in project id: %s" - .formatted(tag.getName(), projectId)); + .formatted(tagName, projectId)); })) .doOnError( throwable -> { - exceptions.put(tag.getName(), throwable); + exceptions.put(tagName, throwable); rethrowExceptionWithLog( throwable, "Final error to delete tag: %s in project id: %s" - .formatted(tag.getName(), projectId)); + .formatted(tagName, projectId)); }) .block(); } @@ -359,7 +407,7 @@ public void deleteTags(String projectId, List tags) { List tagsWithErrors = exceptions.keySet().stream().limit(10).toList(); String andMore = (tagsWithErrors.size() < exceptions.size()) ? " and more." : ""; throw new PhraseClientException( - String.format("Can't delete tags: %s%s", tagsWithErrors, andMore)); + String.format("Can't delete tagNames: %s%s", tagsWithErrors, andMore)); } } diff --git a/webapp/src/test/java/com/box/l10n/mojito/service/thirdparty/phrase/PhraseClientTest.java b/webapp/src/test/java/com/box/l10n/mojito/service/thirdparty/phrase/PhraseClientTest.java index 4b50897c9b..b17a71ab1d 100644 --- a/webapp/src/test/java/com/box/l10n/mojito/service/thirdparty/phrase/PhraseClientTest.java +++ b/webapp/src/test/java/com/box/l10n/mojito/service/thirdparty/phrase/PhraseClientTest.java @@ -1,10 +1,11 @@ package com.box.l10n.mojito.service.thirdparty.phrase; -import com.box.l10n.mojito.service.thirdparty.ThirdPartyTMSPhrase; -import com.google.common.collect.ImmutableList; +import com.box.l10n.mojito.JSR310Migration; import com.phrase.client.model.Tag; -import com.phrase.client.model.TranslationKey; +import java.time.ZonedDateTime; import java.util.List; +import java.util.Objects; +import java.util.stream.Collectors; import org.junit.Assume; import org.junit.Test; import org.junit.runner.RunWith; @@ -36,10 +37,12 @@ public class PhraseClientTest { @Test public void testRemoveTag() { String tagForUpload = "push_2024_06_10_07_17_00_089_122"; - List tagsToDelete = + List tagsToDelete = phraseClient.listTags(testProjectId).stream() .peek(tag -> logger.info("tag: {}", tag)) - .filter(tag -> tag.getName() != null && !tag.getName().equals(tagForUpload)) + .map(Tag::getName) + .filter(Objects::nonNull) + .filter(tagName -> !tagName.equals(tagForUpload)) .toList(); phraseClient.deleteTags(testProjectId, tagsToDelete); @@ -49,27 +52,29 @@ public void testRemoveTag() { public void test() { Assume.assumeNotNull(testProjectId); - String tagForUpload = ThirdPartyTMSPhrase.getTagForUpload(); - - logger.info("tagForUpload: {}", tagForUpload); - - StringBuilder fileContentAndroidBuilder = generateFileContent(); - - String fileContentAndroid = fileContentAndroidBuilder.toString(); - phraseClient.uploadAndWait( - testProjectId, - "en", - "xml", - "strings.xml", - fileContentAndroid, - ImmutableList.of(tagForUpload)); - - phraseClient.removeKeysNotTaggedWith(testProjectId, tagForUpload); - - List translationKeys = phraseClient.getKeys(testProjectId); - for (TranslationKey translationKey : translationKeys) { - logger.info("{}", translationKey); - } + // for (int i = 0; i < 3; i++) { + // String repoName = "repo_%d".formatted(i); + // String tagForUpload = ThirdPartyTMSPhrase.getTagForUpload(repoName); + // + // logger.info("tagForUpload: {}", tagForUpload); + // + // String fileContentAndroid = generateFileContent(repoName).toString(); + // phraseClient.uploadAndWait( + // testProjectId, + // "en", + // "xml", + // "strings.xml", + // fileContentAndroid, + // ImmutableList.of(tagForUpload)); + // + // new ThirdPartyTMSPhrase(phraseClient) + // .removeUnusedKeysAndTags(testProjectId, repoName, tagForUpload); + // } + // + // List translationKeys = phraseClient.getKeys(testProjectId); + // for (TranslationKey translationKey : translationKeys) { + // logger.info("{}", translationKey); + // } // // String fileContentAndroid2 = @@ -87,23 +92,42 @@ public void test() { // """; // phraseClient.uploadCreateFile( // testProjectId, "fr", "xml", "strings.xml", fileContentAndroid2, null); - // String s2 = phraseClient.localeDownload(testProjectId, "fr", "xml"); - // logger.info(s2); + + String s2 = + phraseClient.localeDownload( + testProjectId, + "en", + "xml", + "startWithABadTag", + () -> + phraseClient.listTags(testProjectId).stream() + .map(Tag::getName) + .filter(Objects::nonNull) + .filter(tagName -> tagName.startsWith("push_repo_2")) + .collect(Collectors.joining(","))); + + logger.info(s2); } - static StringBuilder generateFileContent() { + static StringBuilder generateFileContent(String repositoryName) { StringBuilder fileContentAndroidBuilder = new StringBuilder(); fileContentAndroidBuilder.append( """ - Locale Tester - """); + Locale Tester + """ + .formatted(repositoryName)); - for (int i = 0; i < 2000; i++) { + ZonedDateTime now = JSR310Migration.dateTimeNowInUTC(); + for (int i = 0; i < 1; i++) { fileContentAndroidBuilder.append( String.format("Settings\n", i)); + fileContentAndroidBuilder.append( + String.format( + "Settings\n", + repositoryName, i, now.toString())); } fileContentAndroidBuilder.append("");