Handle concurrent run of PhraseTMS sync

Remove Phrase tags only they are older than 5 minutes, this way concurrent sync don't end up in a state where there are no tags, which break the "pull" logic. The time window needs to be higher than the time "push" takes, putting 5 mintues for now.
box · Nov 22, 2024 · 719ec38 · 719ec38
1 parent 1fa2715
commit 719ec38
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 2 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
@@ -27,6 +27,8 @@
 import com.phrase.client.model.TranslationKey;
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.Timer;
+import java.time.Duration;
+import java.time.LocalDateTime;
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
 import java.util.AbstractMap.SimpleEntry;
@@ -52,6 +54,7 @@ public class ThirdPartyTMSPhrase implements ThirdPartyTMS {
 
   static final String TAG_PREFIX = "push_";
   static final String TAG_PREFIX_WITH_REPOSITORY = "push_%s";
+  static final String TAG_DATE_FORMAT = "yyyy_MM_dd_HH_mm_ss_SSS";
   static final boolean NATIVE_CLIENT_DEFAULT_VALUE = false;
 
   static Logger logger = LoggerFactory.getLogger(ThirdPartyTMSPhrase.class);
@@ -249,12 +252,22 @@ public void removeUnusedKeysAndTags(
             .filter(Objects::nonNull)
             .filter(tagName -> tagName.startsWith(TAG_PREFIX))
             .filter(tagName -> !allActiveTags.contains(tagName))
+            // That's to handle concurrent sync and make sure a tag that was just pushed is not
+            // deleted before the  pull from the same sync is finished.
+            // We don't prevent syncs to run concurrently
+            .filter(tagName -> !areTagsWithin5Minutes(tagForUpload, tagName))
             .toList();
 
     logger.info("Tags to delete: {}", pushTagsToDelete);
     phraseClient.deleteTags(projectId, pushTagsToDelete);
+    logger.info("RemoveUnusedKeysAndTags took: {}", stopwatchRemoveUnusedKeysAndTags);
+  }
 
-    logger.info("removeUnusedKeysAndTags took: {}", stopwatchRemoveUnusedKeysAndTags);
+  static boolean areTagsWithin5Minutes(String tagName1, String tagName2) {
+    return Duration.between(uploadTagToLocalDateTime(tagName2), uploadTagToLocalDateTime(tagName1))
+            .abs()
+            .getSeconds()
+        < (60 * 5);
   }
 
   private List<TextUnitDTO> getSourceTextUnitDTOs(
@@ -293,7 +306,7 @@ private List<TextUnitDTO> getSourceTextUnitDTOsPluralOnly(
 
   public static String getTagForUpload(String repositoryName) {
     ZonedDateTime zonedDateTime = JSR310Migration.dateTimeNowInUTC();
-    DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy_MM_dd_HH_mm_ss_SSS");
+    DateTimeFormatter formatter = DateTimeFormatter.ofPattern(TAG_DATE_FORMAT);
     return normalizeTagName(
         "%s%s_%s_%s"
             .formatted(
@@ -303,6 +316,27 @@ public static String getTagForUpload(String repositoryName) {
                 Math.abs(UUID.randomUUID().getLeastSignificantBits() % 1000)));
   }
 
+  public static LocalDateTime uploadTagToLocalDateTime(String tag) {
+
+    if (tag == null || !tag.contains("_")) {
+      throw new IllegalArgumentException("Invalid tag format: " + tag);
+    }
+
+    int dateEndIndex = tag.lastIndexOf('_'); // last part is a random number
+    int dateStartIndex = dateEndIndex;
+
+    for (int i = 0; i < 7; i++) {
+      dateStartIndex = tag.lastIndexOf('_', dateStartIndex - 1);
+      if (dateStartIndex == -1) {
+        throw new IllegalArgumentException("Invalid tag format: " + tag);
+      }
+    }
+
+    String dateTimePart = tag.substring(dateStartIndex + 1, dateEndIndex);
+    DateTimeFormatter formatter = DateTimeFormatter.ofPattern(TAG_DATE_FORMAT);
+    return LocalDateTime.parse(dateTimePart, formatter);
+  }
+
   private static String getTagNamePrefixForRepository(String repositoryName) {
     return normalizeTagName(TAG_PREFIX_WITH_REPOSITORY.formatted(repositoryName));
   }

diff --git a/webapp/src/test/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSPhraseTest.java b/webapp/src/test/java/com/box/l10n/mojito/service/thirdparty/ThirdPartyTMSPhraseTest.java
@@ -1,5 +1,12 @@
 package com.box.l10n.mojito.service.thirdparty;
 
+import static com.box.l10n.mojito.service.thirdparty.ThirdPartyTMSPhrase.areTagsWithin5Minutes;
+import static com.box.l10n.mojito.service.thirdparty.ThirdPartyTMSPhrase.uploadTagToLocalDateTime;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
 import com.box.l10n.mojito.entity.Repository;
 import com.box.l10n.mojito.json.ObjectMapper;
 import com.box.l10n.mojito.service.assetExtraction.ServiceTestBase;
@@ -11,6 +18,8 @@
 import com.box.l10n.mojito.service.tm.search.TextUnitSearcherParameters;
 import com.box.l10n.mojito.test.TestIdWatcher;
 import com.google.common.collect.ImmutableMap;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
 import java.util.List;
 import org.junit.Assume;
 import org.junit.Rule;
@@ -59,6 +68,14 @@ public void testBasics() throws RepositoryLocaleCreationException {
         null,
         null);
 
+    thirdPartyTMSPhrase.push(
+        repository,
+        testProjectId,
+        thirdPartyServiceTestData.getPluralSeparator(),
+        null,
+        null,
+        null);
+
     thirdPartyTMSPhrase.pull(
         repository,
         testProjectId,
@@ -88,4 +105,36 @@ public void testBasics() throws RepositoryLocaleCreationException {
     // t));
 
   }
+
+  @Test
+  public void testUploadTagToLocalDateTimeValid() {
+    LocalDateTime localDateTime = uploadTagToLocalDateTime("push_test_2024_11_21_18_55_38_004_502");
+    assertEquals(
+        "2024-11-21T18:55:38.004",
+        localDateTime.format(DateTimeFormatter.ISO_DATE_TIME),
+        "Parsed LocalDateTime does not match the expected value");
+  }
+
+  @Test
+  public void testUploadTagToLocalDateTimeInvalid() {
+    IllegalArgumentException exception =
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> uploadTagToLocalDateTime("invalid_tag_2024_11_21_18_55_004"));
+    assertTrue(exception.getMessage().contains("Invalid tag format"));
+  }
+
+  @Test
+  public void testTagsWithin5Minutes() {
+    String tag1 = "push_test_2024_11_21_18_55_38_004_502";
+    String tag2 = "push_test_2024_11_21_18_53_30_123_456";
+    assertTrue(areTagsWithin5Minutes(tag1, tag2));
+  }
+
+  @Test
+  public void testTagsOutside5Minutes() {
+    String tag1 = "push_test_2024_11_21_18_55_38_004_502";
+    String tag2 = "push_test_2024_11_21_18_49_30_123_456";
+    assertFalse(areTagsWithin5Minutes(tag1, tag2));
+  }
 }