Skip to content

Commit 8ae6330

Browse files
jglickjglick
authored
Remove idealNames workaround (#479)
2 parents a29e309 + f02c50d commit 8ae6330

File tree

4 files changed

+11
-146
lines changed

4 files changed

+11
-146
lines changed

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

Lines changed: 2 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,14 @@
2525
package com.cloudbees.hudson.plugins.folder;
2626

2727
import com.cloudbees.hudson.plugins.folder.computed.ComputedFolder;
28+
import edu.umd.cs.findbugs.annotations.CheckForNull;
29+
import edu.umd.cs.findbugs.annotations.NonNull;
2830
import hudson.Util;
29-
import hudson.model.AbstractItem;
3031
import hudson.model.Action;
3132
import hudson.model.Item;
32-
import hudson.model.ItemGroup;
3333
import hudson.model.JobProperty;
3434
import hudson.model.TopLevelItem;
35-
import java.io.Closeable;
36-
import java.io.IOException;
37-
import java.util.Map;
38-
import java.util.WeakHashMap;
3935
import java.util.logging.Logger;
40-
import edu.umd.cs.findbugs.annotations.CheckForNull;
41-
import edu.umd.cs.findbugs.annotations.NonNull;
4236
import jenkins.model.TransientActionFactory;
4337

4438
/**
@@ -52,15 +46,6 @@
5246
* <ul>
5347
* <li>See the notes on {@link #itemNameFromItem(AbstractFolder, TopLevelItem)} and
5448
* {@link #dirNameFromItem(AbstractFolder, TopLevelItem)} regarding the constraints on how to name things</li>
55-
* <li>There are some items which need the {@link Item#getRootDir()} during construction (those are bold evil item types
56-
* that leak side-effects, you should fix them if you find them). While you wait for them to be fixed you will need
57-
* to work-around the issue by ensuring that you call {@link #beforeCreateItem(AbstractFolder, String, String)}
58-
* passing the {@link Item#getName()} you want the item to have <strong>and</strong> the ideal unmangled name
59-
* <strong>before</strong> you call {@code new ChildItemType(parent,name)} and then call
60-
* {@link #afterItemCreated(Trace)} when the constructor has returned. Then insure that your
61-
* {@link #itemNameFromItem(AbstractFolder, TopLevelItem)} and {@link #dirNameFromItem(AbstractFolder, TopLevelItem)}
62-
* fall back to {@link #idealNameFromItem(AbstractFolder, TopLevelItem)} when the magic property they are looking
63-
* for is missing.</li>
6449
* </ul>
6550
*
6651
* For a valid implementation, the
@@ -82,58 +67,6 @@
8267
public abstract class ChildNameGenerator<P extends AbstractFolder<I>, I extends TopLevelItem> {
8368
private static final Logger LOGGER = Logger.getLogger(ChildNameGenerator.class.getName());
8469

85-
private static final Map<Trace,String> idealNames = new WeakHashMap<>();
86-
87-
/**
88-
* Work-around helper method to "fix" {@link Item} constructors that have on-disk side-effects and therefore
89-
* need {@link Item#getRootDir()} to work during the constructor.
90-
* @param project the {@link AbstractFolder}.
91-
* @param itemName the name that will be returned by {@link Item#getName()} when the item is constructed. This is
92-
* the second parameter of {@link AbstractItem#AbstractItem(ItemGroup, String)}. This one would be
93-
* the one with URL path segment escaping.
94-
* @param idealName the original name before whatever URL path segment escaping you applied
95-
* @return the {@link Trace} to keep track of when we can remove the memory of the creation process. Please
96-
* {@link Trace#close()} the trace after the item is created.
97-
*/
98-
@NonNull
99-
public static Trace beforeCreateItem(@NonNull AbstractFolder<?> project,
100-
@NonNull String itemName,
101-
@NonNull String idealName) {
102-
final Trace trace = new Trace(project, itemName);
103-
synchronized (idealNames) {
104-
idealNames.put(trace, idealName);
105-
}
106-
return trace;
107-
}
108-
109-
/**
110-
* Clean up for a creation {@link Trace}. Not strictly required, but nice implementations will do this via {@link Trace#close()}.
111-
* @param trace the trace.
112-
*/
113-
private static void afterItemCreated(@NonNull Trace trace) {
114-
synchronized (idealNames) {
115-
idealNames.remove(trace);
116-
}
117-
}
118-
119-
/**
120-
* Looks up the {@link Item} to see if we stored the ideal name before invoking the constructor that is having
121-
* on-disk side-effects before the object has escaped {@link #beforeCreateItem(AbstractFolder, String, String)}
122-
* @param parent the parent within which the item is being created.
123-
* @param item the partially created item.
124-
* @return the ideal name of the item.
125-
*/
126-
@CheckForNull
127-
protected final String idealNameFromItem(@NonNull P parent, @NonNull I item) {
128-
String itemName = item.getName();
129-
if (itemName == null) {
130-
return null;
131-
}
132-
synchronized (idealNames) {
133-
return idealNames.get(new Trace(parent, itemName));
134-
}
135-
}
136-
13770
/**
13871
* Infers the {@link Item#getName()} from the {@link Item} instance itself.
13972
*
@@ -229,66 +162,4 @@ final String dirName(@NonNull P parent, @NonNull I item) {
229162
*/
230163
@NonNull
231164
public abstract String dirNameFromLegacy(@NonNull P parent, @NonNull String legacyDirName);
232-
233-
/**
234-
* Traces the creation of a new {@link Item} in a folder. Use
235-
* {@link ChildNameGenerator#beforeCreateItem(AbstractFolder, String, String)} to get the instance.
236-
*/
237-
public static final class Trace implements Closeable {
238-
/**
239-
* The folder.
240-
*/
241-
@NonNull
242-
private final AbstractFolder<?> folder;
243-
/**
244-
* The {@link Item#getName()} that we expect to be created in the very near future.
245-
*/
246-
@NonNull
247-
private final String itemName;
248-
249-
/**
250-
* Constructor.
251-
* @param folder the folder.
252-
* @param itemName the item name
253-
*/
254-
private Trace(@NonNull AbstractFolder<?> folder, @NonNull String itemName) {
255-
this.folder = folder;
256-
this.itemName = itemName;
257-
}
258-
259-
/**
260-
* {@inheritDoc}
261-
*/
262-
@Override
263-
public boolean equals(Object o) {
264-
if (this == o) {
265-
return true;
266-
}
267-
if (o == null || getClass() != o.getClass()) {
268-
return false;
269-
}
270-
271-
Trace that = (Trace) o;
272-
273-
return folder == that.folder && itemName.equals(that.itemName);
274-
}
275-
276-
/**
277-
* {@inheritDoc}
278-
*/
279-
@Override
280-
public int hashCode() {
281-
int result = folder.hashCode();
282-
result = 31 * result + itemName.hashCode();
283-
return result;
284-
}
285-
286-
/**
287-
* {@inheritDoc}
288-
*/
289-
@Override
290-
public void close() {
291-
afterItemCreated(this);
292-
}
293-
}
294165
}

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -398,9 +398,7 @@ protected void computeChildren(ChildObserver<FreeStyleProject> observer, TaskLis
398398
if (p == null) {
399399
if (observer.mayCreate(encodedKid)) {
400400
listener.getLogger().println("creating a child");
401-
try (ChildNameGenerator.Trace trace = ChildNameGenerator.beforeCreateItem(this, encodedKid, kid)) {
402-
p = new FreeStyleProject(this, encodedKid);
403-
}
401+
p = new FreeStyleProject(this, encodedKid);
404402
BulkChange bc = new BulkChange(p);
405403
try {
406404
p.addProperty(new NameProperty(kid));
@@ -559,7 +557,7 @@ public String itemNameFromItem(@NonNull F parent,
559557
if (property != null) {
560558
return encode(property.getName());
561559
}
562-
String name = idealNameFromItem(parent, item);
560+
String name = item.getName();
563561
return name == null ? null : encode(name);
564562
}
565563

@@ -570,7 +568,7 @@ public String dirNameFromItem(@NonNull F parent,
570568
if (property != null) {
571569
return mangle(property.getName());
572570
}
573-
String name = idealNameFromItem(parent, item);
571+
String name = item.getName();
574572
return name == null ? null : mangle(name);
575573
}
576574

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -326,9 +326,7 @@ protected void computeChildren(ChildObserver<FreeStyleProject> observer, TaskLis
326326
if (p == null) {
327327
if (observer.mayCreate(encodedKid)) {
328328
listener.getLogger().println("creating a child");
329-
try (ChildNameGenerator.Trace trace = ChildNameGenerator.beforeCreateItem(this, encodedKid, kid)) {
330-
p = new FreeStyleProject(this, encodedKid);
331-
}
329+
p = new FreeStyleProject(this, encodedKid);
332330
BulkChange bc = new BulkChange(p);
333331
try {
334332
p.addProperty(new NameProperty(kid));
@@ -460,7 +458,7 @@ public String itemNameFromItem(@NonNull F parent,
460458
if (property != null) {
461459
return encode(property.getName());
462460
}
463-
String name = idealNameFromItem(parent, item);
461+
String name = item.getName();
464462
return name == null ? null : encode(name);
465463
}
466464

@@ -471,7 +469,7 @@ public String dirNameFromItem(@NonNull F parent,
471469
if (property != null) {
472470
return mangle(property.getName());
473471
}
474-
String name = idealNameFromItem(parent, item);
472+
String name = item.getName();
475473
return name == null ? null : mangle(name);
476474
}
477475

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -437,9 +437,7 @@ protected void computeChildren(ChildObserver<FreeStyleProject> observer, TaskLis
437437
if (p == null) {
438438
if (observer.mayCreate(encodedKid)) {
439439
listener.getLogger().println("creating a child");
440-
try (ChildNameGenerator.Trace trace = ChildNameGenerator.beforeCreateItem(this, encodedKid, kid)) {
441-
p = new FreeStyleProject(this, encodedKid);
442-
}
440+
p = new FreeStyleProject(this, encodedKid);
443441
BulkChange bc = new BulkChange(p);
444442
try {
445443
p.addProperty(new NameProperty(kid));
@@ -574,7 +572,7 @@ public String itemNameFromItem(@NonNull F parent,
574572
if (property != null) {
575573
return encode(property.getName());
576574
}
577-
String name = idealNameFromItem(parent, item);
575+
String name = item.getName();
578576
return name == null ? null : encode(name);
579577
}
580578

@@ -585,7 +583,7 @@ public String dirNameFromItem(@NonNull F parent,
585583
if (property != null) {
586584
return mangle(property.getName());
587585
}
588-
String name = idealNameFromItem(parent, item);
586+
String name = item.getName();
589587
return name == null ? null : mangle(name);
590588
}
591589

0 commit comments

Comments
 (0)