From 915a151970559c7f3015e799f5ab27e4b22e62b3 Mon Sep 17 00:00:00 2001 From: Jean Aurambault Date: Sat, 22 Jun 2024 14:26:40 -0700 Subject: [PATCH] Fix issues related to tag names having some character converted --- .../service/thirdparty/ThirdPartyService.java | 9 ++- .../thirdparty/ThirdPartyTMSPhrase.java | 59 ++++++++++++++----- .../ThridPartyTMSPhraseException.java | 4 ++ .../thirdparty/phrase/PhraseClient.java | 23 ++++++-- 4 files changed, 74 insertions(+), 21 deletions(-) diff --git a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyService.java b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyService.java index d5e5c68ba0..f8f4f3d0c5 100644 --- a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyService.java +++ b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyService.java @@ -278,7 +278,14 @@ void mapMojitoAndThirdPartyTextUnits( thirdPartyTextUnits.stream() .collect( groupingBy( - o -> assetCache.getUnchecked(o.getAssetPath()).orElse(null), + o -> + assetCache + .getUnchecked(o.getAssetPath()) + .orElseThrow( + () -> + new RuntimeException( + "Trying to map a third party text unit for an asset (%s) that does not exist in the repository" + .formatted(o.getAssetPath()))), LinkedHashMap::new, toList())); 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 b511217dc8..a0993ca715 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 @@ -80,7 +80,10 @@ public List getThirdPartyTextUnits( List thirdPartyTextUnits = new ArrayList<>(); - List phraseTranslationKeys = phraseClient.getKeys(projectId); + String currentTagsForRepository = getCurrentTagsForRepository(repository, projectId); + + List phraseTranslationKeys = + phraseClient.getKeys(projectId, currentTagsForRepository); for (TranslationKey translationKey : phraseTranslationKeys) { @@ -181,9 +184,7 @@ public void removeUnusedKeysAndTags( .map(Tag::getName) .filter(Objects::nonNull) .filter(tagName -> tagName.startsWith(TAG_PREFIX)) - .filter( - tagName -> - !tagName.startsWith(TAG_PREFIX_WITH_REPOSITORY.formatted(repositoryName))) + .filter(tagName -> !tagName.startsWith(getTagNamePrefixForRepository(repositoryName))) .toList(); List allActiveTags = new ArrayList<>(tagsForOtherRepositories); @@ -241,12 +242,25 @@ private List getSourceTextUnitDTOsPluralOnly( 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_%s") - .formatted( - TAG_PREFIX, - repositoryName, - formatter.format(zonedDateTime), - Math.abs(UUID.randomUUID().getLeastSignificantBits() % 1000)); + return normalizeTagName( + "%s%s_%s_%s" + .formatted( + TAG_PREFIX, + repositoryName, + formatter.format(zonedDateTime), + Math.abs(UUID.randomUUID().getLeastSignificantBits() % 1000))); + } + + private static String getTagNamePrefixForRepository(String repositoryName) { + return normalizeTagName(TAG_PREFIX_WITH_REPOSITORY.formatted(repositoryName)); + } + + /** + * At least "/" are getting converted by phrase into "_", apply that logic on our side to have + * consistent filtering + */ + private static String normalizeTagName(String repositoryName) { + return repositoryName.replace("/", "_"); } @Override @@ -268,7 +282,7 @@ public PollableFuture pull( for (RepositoryLocale repositoryLocale : repositoryLocalesWithoutRootLocale) { String localeTag = repositoryLocale.getLocale().getBcp47Tag(); - logger.info("Downloading locale: {} from Phrase", localeTag); + logger.info("Downloading locale: {} from Phrase with tags: {}", localeTag, currentTags); String fileContent = phraseClient.localeDownload( @@ -293,12 +307,25 @@ public PollableFuture pull( return null; } + /** + * It should typically return a single tag, but if concurrent syncs, it could return more. That + * should not be a problem when downloading or keys for mapping or pulling strings + */ 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(",")); + String tagNamePrefixForRepository = getTagNamePrefixForRepository(repository.getName()); + String tags = + phraseClient.listTags(projectId).stream() + .map(Tag::getName) + .filter(Objects::nonNull) + .filter(tagName -> tagName.startsWith(tagNamePrefixForRepository)) + .collect(Collectors.joining(",")); + + if (tags.isBlank()) { + throw new RuntimeException( + "There are no current tags for the repository, make sure push was run first or that the repository name does not contain special character (which will need to get normalized)"); + } + + return tags; } @Override diff --git a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThridPartyTMSPhraseException.java b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThridPartyTMSPhraseException.java index 835c16c069..1756f5b2a6 100644 --- a/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThridPartyTMSPhraseException.java +++ b/webapp/src/main/java/com/box/l10n/mojito/service/thirdparty/ThridPartyTMSPhraseException.java @@ -4,4 +4,8 @@ public class ThridPartyTMSPhraseException extends RuntimeException { public ThridPartyTMSPhraseException(String msg, Throwable e) { super(msg, e); } + + public ThridPartyTMSPhraseException(String message) { + super(message); + } } 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 d5ffd75a27..831cca200b 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 @@ -105,16 +105,23 @@ String uploadCreateFile( Path tmpWorkingDirectory = null; + logger.info( + "uploadCreateFile: projectId: {}, localeId: {}, fileName: {}, tags: {}", + projectId, + localeId, + fileName, + tags); + try { tmpWorkingDirectory = createTempDirectory("phrase-integration"); if (tmpWorkingDirectory.toFile().exists()) { - logger.info("Created temporary working directory: {}", tmpWorkingDirectory); + logger.debug("Created temporary working directory: {}", tmpWorkingDirectory); } Path fileToUpload = tmpWorkingDirectory.resolve(fileName); - logger.info("Create file: {}", fileToUpload); + logger.debug("Create file: {}", fileToUpload); createDirectories(fileToUpload.getParent()); write(fileToUpload, fileContent); @@ -294,7 +301,7 @@ public String localeDownload( .block(); } - public List getKeys(String projectId) { + public List getKeys(String projectId, String tags) { KeysApi keysApi = new KeysApi(apiClient); AtomicInteger page = new AtomicInteger(0); int batchSize = BATCH_SIZE; @@ -305,7 +312,15 @@ public List getKeys(String projectId) { () -> { logger.info("Fetching keys for project: {}, page: {}", projectId, page); return keysApi.keysList( - projectId, null, page.get(), batchSize, null, null, null, null, null); + projectId, + null, + page.get(), + batchSize, + null, + null, + null, + "tags:%s".formatted(tags), + null); }) .retryWhen( retryBackoffSpec.doBeforeRetry(