Remove ChildNameGenerator.CHILD_NAME_FILE

Author: basil

src/main/java/com/cloudbees/hudson/plugins/folder/AbstractFolder.java

@@ -360,13 +360,10 @@ public static <K, V extends TopLevelItem> Map<K, V> loadChildren(AbstractFolder<
         }
         for (File subdir : subdirs) {
             File effectiveSubdir = subdir;
-            String childName = childNameGenerator.readItemName(subdir);
             // Try to retain the identity of an existing child object if we can.
             V itemFromDir;
             V item;
-            String name;
-            var legacyName = subdir.getName();
-            item = itemFromDir = byDirName.get(legacyName);
+            item = itemFromDir = byDirName.get(subdir.getName());
             try {
                 if (item == null) {
                     XmlFile xmlFile = Items.getConfigFile(subdir);
@@ -377,19 +374,15 @@ public static <K, V extends TopLevelItem> Map<K, V> loadChildren(AbstractFolder<
                         throw new FileNotFoundException("Could not find configuration file " + xmlFile.getFile());
                     }
                 }
-                name = childNameGenerator.writeItemName(parent, item, effectiveSubdir, childName);
-                String computedName = childNameGenerator.itemNameFromItem(parent, item);
-                if (computedName == null) {
-                    computedName = subdir.getName();
-                }
-                if (!computedName.equals(name)) {
-                    throw new IllegalStateException("Computed name '" + computedName + "' does not match name written to file '" + name + "'");
+                String name = childNameGenerator.itemNameFromItem(parent, item);
+                if (name == null) {
+                    name = subdir.getName();
                 }
                 if (item instanceof AbstractItem) {
                     var abstractItem = (AbstractItem) item;
-                    if (itemFromDir != null && !legacyName.equals(name) && abstractItem.getDisplayNameOrNull() == null) {
+                    if (itemFromDir != null && !subdir.getName().equals(name) && abstractItem.getDisplayNameOrNull() == null) {
                         try (BulkChange ignored = new BulkChange(item)) {
-                            abstractItem.setDisplayName(childName);
+                            abstractItem.setDisplayName(name);
                         }
                     }
                 }
@@ -407,22 +400,13 @@ public static <K, V extends TopLevelItem> Map<K, V> loadChildren(AbstractFolder<
 
     @Override
     public String getItemName(File dir, I item) {
-        String name = childNameGenerator().readItemName(dir);
-        String computedName = childNameGenerator().itemNameFromItem(this, item);
-        if (computedName == null) {
-            computedName = dir.getName();
-        }
-        if (!computedName.equals(name)) {
-            throw new IllegalStateException("Computed name '" + computedName + "' does not match name read from file '" + name + "'");
+        String name = childNameGenerator().itemNameFromItem(this, item);
+        if (name == null) {
+            name = dir.getName();
         }
         return name;
     }
 
-    protected final I itemsPut(String name, I item) {
-        childNameGenerator().writeItemName(this, item, getRootDirFor(item), name);
-        return items.put(name, item);
-    }
-
     /**
      * Reloads this folder itself.
      * Compared to {@link #load}, this method skips parts of {@link #onLoad} such as the call to {@link #loadChildren}.
@@ -453,7 +437,7 @@ public void addLoadedChild(I item, String name) throws IOException, IllegalArgum
         if (items.containsKey(name)) {
             throw new IllegalArgumentException("already an item '" + name + "'");
         }
-        itemsPut(item.getName(), item);
+        items.put(item.getName(), item);
     }
 
     /**
@@ -1032,7 +1016,7 @@ public I getItem(String name) throws AccessDeniedException {
     @Override
     public void onRenamed(I item, String oldName, String newName) throws IOException {
         items.remove(oldName);
-        itemsPut(newName, item);
+        items.put(newName, item);
         // For compatibility with old views:
         for (View v : folderViews.getViews()) {
             v.onJobRenamed(item, oldName, newName);

src/main/java/com/cloudbees/hudson/plugins/folder/ChildNameGenerator.java

@@ -35,17 +35,13 @@
 import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
-import java.io.UncheckedIOException;
-import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
 import java.util.Map;
 import java.util.WeakHashMap;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import jenkins.model.TransientActionFactory;
-import org.apache.commons.lang.StringUtils;
 
 /**
  * Provides a way for a {@link ComputedFolder} to break the association between the directory names on disk
@@ -87,17 +83,6 @@
  */
 public abstract class ChildNameGenerator<P extends AbstractFolder<I>, I extends TopLevelItem> {
     private static final Logger LOGGER = Logger.getLogger(ChildNameGenerator.class.getName());
-    /**
-     * The name of the file that contains the actual name of the child item. This file is to allow a Jenkins
-     * Administrator to determine which child is which when dealing with a folder containing child names that have
-     * been mangled.
-     * <p>
-     * If there is nothing else to go on, this file will be used in preference to the child directory name, but as it
-     * is too easy for users to mistakenly think changing the contents of the file will rename the child (which could
-     * cause data loss for the computed folder's child) it is better for implementations to store the definitive
-     * ideal name in a {@link JobProperty}, {@link Action} or equivalent that is attached directly to the {@link Item}.
-     */
-    public static final String CHILD_NAME_FILE = "name-utf8.txt";
 
     private static final Map<Trace,String> idealNames = new WeakHashMap<>();
 
@@ -289,61 +274,6 @@ final File ensureItemDirectory(@NonNull P parent, @NonNull I item, @NonNull File
      */
     public abstract void recordLegacyName(P parent, I item, String legacyDirName) throws IOException;
 
-    /**
-     * Reads an item name from the given directory.
-     * @param directory the directory containing the item.
-     * @return The item name obtained from the directory, or the directory name if the name file is missing or empty.
-     */
-    @NonNull
-    public final String readItemName(@NonNull File directory) {
-        String childName = directory.getName();
-        File nameFile = new File(directory, CHILD_NAME_FILE);
-        if (nameFile.isFile()) {
-            try {
-                childName = StringUtils.defaultString(StringUtils.trimToNull(Files.readString(nameFile.toPath(), StandardCharsets.UTF_8)), directory.getName());
-            } catch (IOException e) {
-                LOGGER.log(Level.WARNING, () -> "Could not read "+ nameFile + ", assuming child name is " + directory.getName());
-            }
-        }
-        return childName;
-    }
-
-    /**
-     * Writes the item name to the given directory.
-     * @param parent The parent folder of the item.
-     * @param item The item we want to write the name for.
-     * @param itemDirectory The directory where the item is stored.
-     * @param childName The desired name for the item.
-     * @return The name that was written to the directory, and whether the item needs to be saved.
-     */
-    @NonNull
-    public final String writeItemName(@NonNull P parent, @NonNull I item, @NonNull File itemDirectory, @NonNull String childName) {
-        String name = itemNameFromItem(parent, item);
-        if (name == null) {
-            name = itemNameFromLegacy(parent, childName);
-        }
-        File nameFile = new File(itemDirectory, CHILD_NAME_FILE);
-        try {
-            var itemPath = itemDirectory.toPath();
-            if (Files.notExists(itemPath)) {
-                Files.createDirectories(itemPath);
-            }
-            String existingName;
-            if (Files.exists(nameFile.toPath())) {
-                existingName = Files.readString(nameFile.toPath(), StandardCharsets.UTF_8);
-            } else {
-                existingName = null;
-            }
-            if (existingName == null || !existingName.equals(name)) {
-                Files.writeString(nameFile.toPath(), name, StandardCharsets.UTF_8);
-            }
-        } catch (IOException e) {
-            // Unfortunately not all callers of this method throw IOException, so we need to go unchecked
-            throw new UncheckedIOException("Failed to load " + name + " as could not write " + nameFile, e);
-        }
-        return name;
-    }
-
     /**
      * Traces the creation of a new {@link Item} in a folder. Use
      * {@link ChildNameGenerator#beforeCreateItem(AbstractFolder, String, String)} to get the instance.

src/main/java/com/cloudbees/hudson/plugins/folder/Folder.java

@@ -457,7 +457,7 @@ private MixInImpl(Folder parent) {
 
         @Override
         protected void add(TopLevelItem item) {
-            itemsPut(item.getName(), item);
+            items.put(item.getName(), item);
         }
 
         @Override

src/main/java/com/cloudbees/hudson/plugins/folder/computed/ComputedFolder.java

@@ -870,7 +870,7 @@ public void created(I child) {
             } catch (IOException x) {
                 LOGGER.log(Level.WARNING, "Failed to save " + child, x);
             }
-            itemsPut(name, child);
+            items.put(name, child);
             Jenkins j = Jenkins.getInstanceOrNull();
             if (j != null) {
                 j.rebuildDependencyGraphAsync();
@@ -991,7 +991,7 @@ public void created(I child) {
             } catch (IOException x) {
                 LOGGER.log(Level.WARNING, "Failed to save " + child, x);
             }
-            itemsPut(name, child);
+            items.put(name, child);
             Jenkins j = Jenkins.getInstanceOrNull();
             if (j != null) {
                 j.rebuildDependencyGraphAsync();

src/test/java/com/cloudbees/hudson/plugins/folder/ChildNameGeneratorAltTest.java

@@ -40,11 +40,9 @@
 import hudson.model.TaskListener;
 import hudson.model.TopLevelItem;
 import java.io.ByteArrayOutputStream;
-import java.io.File;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
 import java.text.Normalizer;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -246,12 +244,6 @@ private void checkChild(ComputedFolderImpl instance, String idealName) throws IO
         assertThat("We have an item for name " + idealName, item, notNullValue());
         assertThat("The root directory if the item for name " + idealName + " is mangled",
                 item.getRootDir().getName(), is(mangle(idealName)));
-        File nameFile = new File(item.getRootDir(), ChildNameGenerator.CHILD_NAME_FILE);
-        assertThat("We have the " + ChildNameGenerator.CHILD_NAME_FILE + " for the item for name " + idealName,
-                nameFile.isFile(), is(true));
-        String name = Files.readString(nameFile.toPath(), StandardCharsets.UTF_8);
-        assertThat("The " + ChildNameGenerator.CHILD_NAME_FILE + " for the item for name " + idealName
-                + " contains the encoded name", name, is(encodedName));
     }
 
     public static String encode(String s) {

src/test/java/com/cloudbees/hudson/plugins/folder/ChildNameGeneratorRecTest.java

@@ -40,10 +40,8 @@
 import hudson.model.TaskListener;
 import hudson.model.TopLevelItem;
 import java.io.ByteArrayOutputStream;
-import java.io.File;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
 import java.text.Normalizer;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -177,12 +175,6 @@ private void checkChild(ComputedFolderImpl instance, String idealName) throws IO
         assertThat("We have an item for name " + idealName, item, notNullValue());
         assertThat("The root directory of the item for name " + idealName + " is mangled",
                 item.getRootDir().getName(), is(mangle(idealName)));
-        File nameFile = new File(item.getRootDir(), ChildNameGenerator.CHILD_NAME_FILE);
-        assertThat("We have the " + ChildNameGenerator.CHILD_NAME_FILE + " for the item for name " + idealName,
-                nameFile.isFile(), is(true));
-        String name = Files.readString(nameFile.toPath(), StandardCharsets.UTF_8);
-        assertThat("The " + ChildNameGenerator.CHILD_NAME_FILE + " for the item for name " + idealName
-                + " contains the encoded name", name, is(encodedName));
     }
 
     public static String encode(String s) {

src/test/java/com/cloudbees/hudson/plugins/folder/ChildNameGeneratorTest.java

@@ -43,7 +43,6 @@
 import java.io.File;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
 import java.text.Normalizer;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -314,12 +313,6 @@ private void checkChild(ComputedFolderImpl instance, String idealName) throws IO
             assertThat("Alternative normalized form: " + altRootDir + " does not exist",
                     altRootDir.isDirectory(), is(false));
         }
-        File nameFile = new File(item.getRootDir(), ChildNameGenerator.CHILD_NAME_FILE);
-        assertThat("We have the " + ChildNameGenerator.CHILD_NAME_FILE + " for the item for name " + idealName,
-                nameFile.isFile(), is(true));
-        String name = Files.readString(nameFile.toPath(), StandardCharsets.UTF_8);
-        assertThat("The " + ChildNameGenerator.CHILD_NAME_FILE + " for the item for name " + idealName
-                + " contains the encoded name", name, is(encodedName));
     }
 
     public static String encode(String s) {