Skip to content

Commit f5b020d

Browse files
committed
Aggressively convert potential inputs
If we do not run conversion pre-emptively, then input lists will potentially: - have duplicate items (multiples of which will convert to the same object) - change between runs (first run will show pre-converted item, second+ run will have post-converted item)
1 parent 0d95340 commit f5b020d

File tree

2 files changed

+11
-33
lines changed

2 files changed

+11
-33
lines changed

src/main/java/org/scijava/widget/AbstractInputHarvester.java

+10-5
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,10 @@
3030
package org.scijava.widget;
3131

3232
import java.util.ArrayList;
33-
import java.util.HashSet;
33+
import java.util.Collection;
3434
import java.util.List;
3535
import java.util.Set;
36+
import java.util.stream.Collectors;
3637

3738
import org.scijava.AbstractContextual;
3839
import org.scijava.convert.ConvertService;
@@ -129,9 +130,13 @@ private <T> WidgetModel addInput(final InputPanel<P, W> inputPanel,
129130

130131
/** Asks the object service and convert service for valid choices */
131132
private List<Object> getObjects(final Class<?> type) {
132-
Set<Object> compatibleInputs =
133-
new HashSet<>(convertService.getCompatibleInputs(type));
134-
compatibleInputs.addAll(objectService.getObjects(type));
135-
return new ArrayList<>(compatibleInputs);
133+
// Here we compare with object service's items and de-duplicate
134+
Collection<Object> compatibleInputs = convertService.getCompatibleInputs(type);
135+
// NB: we aggressively convert here to avoid listing duplicate items
136+
// that would effectively resolve to the same object
137+
Set<Object> objects = compatibleInputs.stream()
138+
.map(o -> convertService.convert(o, type)).collect(Collectors.toSet());
139+
objects.addAll(objectService.getObjects(type));
140+
return new ArrayList<>(objects);
136141
}
137142
}

src/main/java/org/scijava/widget/DefaultWidgetModel.java

+1-28
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,7 @@
3030
package org.scijava.widget;
3131

3232
import java.util.List;
33-
import java.util.Map;
3433
import java.util.Objects;
35-
import java.util.WeakHashMap;
3634

3735
import org.scijava.AbstractContextual;
3836
import org.scijava.Context;
@@ -60,7 +58,6 @@ public class DefaultWidgetModel extends AbstractContextual implements WidgetMode
6058
private final Module module;
6159
private final ModuleItem<?> item;
6260
private final List<?> objectPool;
63-
private final Map<Object, Object> convertedObjects;
6461

6562
@Parameter
6663
private ThreadService threadService;
@@ -87,7 +84,6 @@ public DefaultWidgetModel(final Context context, final InputPanel<?, ?> inputPan
8784
this.module = module;
8885
this.item = item;
8986
this.objectPool = objectPool;
90-
convertedObjects = new WeakHashMap<>();
9187

9288
if (item.getValue(module) == null) {
9389
// assign the item's default value as the current value
@@ -142,27 +138,9 @@ public Object getValue() {
142138

143139
@Override
144140
public void setValue(final Object value) {
145-
final String name = item.getName();
146141
if (Objects.equals(item.getValue(module), value)) return; // no change
147142

148-
// Check if a converted value is present
149-
Object convertedInput = convertedObjects.get(value);
150-
if (convertedInput != null &&
151-
Objects.equals(item.getValue(module), convertedInput))
152-
{
153-
return; // no change
154-
}
155-
156-
// Pass the value through the convertService
157-
convertedInput = convertService.convert(value, item.getType());
158-
if (convertedInput == null) convertedInput = value;
159-
160-
// If we get a different (converted) value back, cache it weakly.
161-
if (convertedInput != value) {
162-
convertedObjects.put(value, convertedInput);
163-
}
164-
165-
module.setInput(name, convertedInput);
143+
module.setInput(item.getName(), value);
166144

167145
if (initialized) {
168146
threadService.queue(() -> {
@@ -309,11 +287,6 @@ private Object ensureValidObject(final Object value) {
309287
private Object ensureValid(final Object value, final List<?> list) {
310288
for (final Object o : list) {
311289
if (o.equals(value)) return value; // value is valid
312-
// check if value was converted and cached
313-
final Object convertedValue = convertedObjects.get(o);
314-
if (convertedValue != null && value.equals(convertedValue)) {
315-
return convertedValue;
316-
}
317290
}
318291

319292
// value is not valid; override with the first item on the list instead

0 commit comments

Comments
 (0)