Improve translations handling (#3166)

common/src/main/java/me/lucko/luckperms/common/locale/TranslationManager.java

@@ -28,6 +28,7 @@ package me.lucko.luckperms.common.locale;
 import com.google.common.collect.Maps;
 
 import me.lucko.luckperms.common.plugin.LuckPermsPlugin;
+import me.lucko.luckperms.common.util.MoreFiles;
 
 import net.kyori.adventure.key.Key;
 import net.kyori.adventure.text.Component;
@@ -60,19 +61,39 @@ public class TranslationManager {
     public static final Locale DEFAULT_LOCALE = Locale.ENGLISH;
 
     private final LuckPermsPlugin plugin;
-    private final Path translationsDirectory;
     private final Set<Locale> installed = ConcurrentHashMap.newKeySet();
     private TranslationRegistry registry;
 
+    private final Path translationsDirectory;
+    private final Path repositoryTranslationsDirectory;
+    private final Path customTranslationsDirectory;
+
     public TranslationManager(LuckPermsPlugin plugin) {
         this.plugin = plugin;
         this.translationsDirectory = this.plugin.getBootstrap().getConfigDirectory().resolve("translations");
+        this.repositoryTranslationsDirectory = this.translationsDirectory.resolve("repository");
+        this.customTranslationsDirectory = this.translationsDirectory.resolve("custom");
+
+        try {
+            MoreFiles.createDirectoriesIfNotExists(this.repositoryTranslationsDirectory);
+            MoreFiles.createDirectoriesIfNotExists(this.customTranslationsDirectory);
+        } catch (IOException e) {
+            // ignore
+        }
     }
 
     public Path getTranslationsDirectory() {
         return this.translationsDirectory;
     }
 
+    public Path getRepositoryTranslationsDirectory() {
+        return this.repositoryTranslationsDirectory;
+    }
+
+    public Path getRepositoryStatusFile() {
+        return this.repositoryTranslationsDirectory.resolve("status.json");
+    }
+
     public Set<Locale> getInstalledLocales() {
         return Collections.unmodifiableSet(this.installed);
     }
@@ -89,8 +110,9 @@ public class TranslationManager {
         this.registry.defaultLocale(DEFAULT_LOCALE);
 
         // load custom translations first, then the base (built-in) translations after.
-        loadCustom();
+        loadFromFileSystem(this.customTranslationsDirectory, false);
-        loadBase();
+        loadFromFileSystem(this.repositoryTranslationsDirectory, true);
+        loadFromResourceBundle();
 
         // register it to the global source, so our translations can be picked up by adventure-platform
         GlobalTranslator.get().addSource(this.registry);
@@ -99,36 +121,45 @@ public class TranslationManager {
     /**
      * Loads the base (English) translations from the jar file.
      */
-    private void loadBase() {
+    private void loadFromResourceBundle() {
         ResourceBundle bundle = ResourceBundle.getBundle("luckperms", DEFAULT_LOCALE, UTF8ResourceBundleControl.get());
         try {
             this.registry.registerAll(DEFAULT_LOCALE, bundle, false);
         } catch (IllegalArgumentException e) {
-            this.plugin.getLogger().warn("Error loading default locale file", e);
+            if (!isAdventureDuplicatesException(e)) {
+                this.plugin.getLogger().warn("Error loading default locale file", e);
+            }
         }
     }
 
+    public static boolean isTranslationFile(Path path) {
+        return path.getFileName().toString().endsWith(".properties");
+    }
+
     /**
      * Loads custom translations (in any language) from the plugin configuration folder.
      */
-    public void loadCustom() {
+    public void loadFromFileSystem(Path directory, boolean suppressDuplicatesError) {
         List<Path> translationFiles;
-        try (Stream<Path> stream = Files.list(this.translationsDirectory)) {
+        try (Stream<Path> stream = Files.list(directory)) {
-            translationFiles = stream.filter(path -> path.getFileName().toString().endsWith(".properties")).collect(Collectors.toList());
+            translationFiles = stream.filter(TranslationManager::isTranslationFile).collect(Collectors.toList());
         } catch (IOException e) {
             translationFiles = Collections.emptyList();
         }
 
+        if (translationFiles.isEmpty()) {
+            return;
+        }
+
         Map<Locale, ResourceBundle> loaded = new HashMap<>();
         for (Path translationFile : translationFiles) {
             try {
-                Map.Entry<Locale, ResourceBundle> result = loadCustomTranslationFile(translationFile);
+                Map.Entry<Locale, ResourceBundle> result = loadTranslationFile(translationFile);
                 loaded.put(result.getKey(), result.getValue());
-            } catch (IllegalArgumentException e) {
-                // common error is from adventure "java.lang.IllegalArgumentException: Invalid key" -- don't print the whole stack trace.
-                this.plugin.getLogger().warn("Error loading locale file: " + translationFile.getFileName() + " - " + e);
             } catch (Exception e) {
-                this.plugin.getLogger().warn("Error loading locale file: " + translationFile.getFileName(), e);
+                if (!suppressDuplicatesError || !isAdventureDuplicatesException(e)) {
+                    this.plugin.getLogger().warn("Error loading locale file: " + translationFile.getFileName(), e);
+                }
             }
         }
 
@@ -139,13 +170,13 @@ public class TranslationManager {
                 try {
                     this.registry.registerAll(localeWithoutCountry, bundle, false);
                 } catch (IllegalArgumentException e) {
-                    // ignore "IllegalArgumentException: Invalid key" from adventure TranslationRegistry
+                    // ignore
                 }
             }
         });
     }
 
-    private Map.Entry<Locale, ResourceBundle> loadCustomTranslationFile(Path translationFile) throws IOException {
+    private Map.Entry<Locale, ResourceBundle> loadTranslationFile(Path translationFile) throws IOException {
         String fileName = translationFile.getFileName().toString();
         String localeString = fileName.substring(0, fileName.length() - ".properties".length());
         Locale locale = parseLocale(localeString);
@@ -164,6 +195,11 @@ public class TranslationManager {
         return Maps.immutableEntry(locale, bundle);
     }
 
+    @SuppressWarnings("BooleanMethodIsAlwaysInverted")
+    private static boolean isAdventureDuplicatesException(Exception e) {
+        return e instanceof IllegalArgumentException && (e.getMessage().startsWith("Invalid key") || e.getMessage().startsWith("Translation already exists"));
+    }
+
     public static Component render(Component component) {
         return render(component, Locale.getDefault());
     }

common/src/main/java/me/lucko/luckperms/common/locale/TranslationRepository.java

@@ -33,7 +33,6 @@ import me.lucko.luckperms.common.config.ConfigKeys;
 import me.lucko.luckperms.common.http.UnsuccessfulRequestException;
 import me.lucko.luckperms.common.plugin.LuckPermsPlugin;
 import me.lucko.luckperms.common.sender.Sender;
-import me.lucko.luckperms.common.util.MoreFiles;
 import me.lucko.luckperms.common.util.gson.GsonProvider;
 
 import org.checkerframework.checker.nullness.qual.Nullable;
@@ -59,6 +58,7 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Predicate;
 
 public class TranslationRepository {
     private static final String TRANSLATIONS_INFO_ENDPOINT = "https://metadata.luckperms.net/data/translations";
@@ -92,6 +92,9 @@ public class TranslationRepository {
         }
 
         this.plugin.getBootstrap().getScheduler().executeAsync(() -> {
+            // cleanup old translation files
+            clearDirectory(this.plugin.getTranslationManager().getTranslationsDirectory(), Files::isRegularFile);
+
             try {
                 refresh();
             } catch (Exception e) {
@@ -101,34 +104,14 @@ public class TranslationRepository {
     }
 
     private void refresh() throws Exception {
-        Path translationsDirectory = this.plugin.getTranslationManager().getTranslationsDirectory();
+        long lastRefresh = readLastRefreshTime();
-        try {
-            MoreFiles.createDirectoriesIfNotExists(translationsDirectory);
-        } catch (IOException e) {
-            // ignore
-        }
-
-        long lastRefresh = 0L;
-
-        Path repoStatusFile = translationsDirectory.resolve("repository.json");
-        if (Files.exists(repoStatusFile)) {
-            try (BufferedReader reader = Files.newBufferedReader(repoStatusFile, StandardCharsets.UTF_8)) {
-                JsonObject status = GsonProvider.normal().fromJson(reader, JsonObject.class);
-                if (status.has("lastRefresh")) {
-                    lastRefresh = status.get("lastRefresh").getAsLong();
-                }
-            } catch (Exception e) {
-                // ignore
-            }
-        }
-
         long timeSinceLastRefresh = System.currentTimeMillis() - lastRefresh;
+
         if (timeSinceLastRefresh <= CACHE_MAX_AGE) {
             return;
         }
 
         MetadataResponse metadata = getTranslationsMetadata();
-
         if (timeSinceLastRefresh <= metadata.cacheMaxAge) {
             return;
         }
@@ -137,6 +120,22 @@ public class TranslationRepository {
         downloadAndInstallTranslations(metadata.languages, null, true);
     }
 
+    private void clearDirectory(Path directory, Predicate<Path> predicate) {
+        try {
+            Files.list(directory)
+                    .filter(predicate)
+                    .forEach(p -> {
+                        try {
+                            Files.delete(p);
+                        } catch (IOException e) {
+                            // ignore
+                        }
+                    });
+        } catch (IOException e) {
+            // ignore
+        }
+    }
+
     /**
      * Downloads and installs translations for the given languages.
      *
@@ -146,13 +145,10 @@ public class TranslationRepository {
      */
     public void downloadAndInstallTranslations(List<LanguageInfo> languages, @Nullable Sender sender, boolean updateStatus) {
         TranslationManager manager = this.plugin.getTranslationManager();
-        Path translationsDirectory = manager.getTranslationsDirectory();
+        Path translationsDirectory = manager.getRepositoryTranslationsDirectory();
 
-        try {
+        // clear existing translations
-            MoreFiles.createDirectoriesIfNotExists(translationsDirectory);
+        clearDirectory(translationsDirectory, TranslationManager::isTranslationFile);
-        } catch (IOException e) {
-            // ignore
-        }
 
         for (LanguageInfo language : languages) {
             if (sender != null) {
@@ -185,18 +181,39 @@ public class TranslationRepository {
         }
 
         if (updateStatus) {
-            // update status file
+            writeLastRefreshTime();
-            Path repoStatusFile = translationsDirectory.resolve("repository.json");
+        }
-            try (BufferedWriter writer = Files.newBufferedWriter(repoStatusFile, StandardCharsets.UTF_8)) {
+
-                JsonObject status = new JsonObject();
+        manager.reload();
-                status.add("lastRefresh", new JsonPrimitive(System.currentTimeMillis()));
+    }
-                GsonProvider.prettyPrinting().toJson(status, writer);
+
-            } catch (IOException e) {
+    private void writeLastRefreshTime() {
+        Path statusFile = this.plugin.getTranslationManager().getRepositoryStatusFile();
+
+        try (BufferedWriter writer = Files.newBufferedWriter(statusFile, StandardCharsets.UTF_8)) {
+            JsonObject status = new JsonObject();
+            status.add("lastRefresh", new JsonPrimitive(System.currentTimeMillis()));
+            GsonProvider.prettyPrinting().toJson(status, writer);
+        } catch (IOException e) {
+            // ignore
+        }
+    }
+
+    private long readLastRefreshTime() {
+        Path statusFile = this.plugin.getTranslationManager().getRepositoryStatusFile();
+
+        if (Files.exists(statusFile)) {
+            try (BufferedReader reader = Files.newBufferedReader(statusFile, StandardCharsets.UTF_8)) {
+                JsonObject status = GsonProvider.normal().fromJson(reader, JsonObject.class);
+                if (status.has("lastRefresh")) {
+                    return status.get("lastRefresh").getAsLong();
+                }
+            } catch (Exception e) {
                 // ignore
             }
         }
 
-        this.plugin.getTranslationManager().reload();
+        return 0L;
     }
 
     private MetadataResponse getTranslationsMetadata() throws IOException, UnsuccessfulRequestException {