Skip to content

Commit

Permalink
Aggressively convert potential inputs
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
hinerm committed Sep 6, 2024
1 parent 0d95340 commit f5b020d
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 33 deletions.
15 changes: 10 additions & 5 deletions src/main/java/org/scijava/widget/AbstractInputHarvester.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@
package org.scijava.widget;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

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

/** Asks the object service and convert service for valid choices */
private List<Object> getObjects(final Class<?> type) {
Set<Object> compatibleInputs =
new HashSet<>(convertService.getCompatibleInputs(type));
compatibleInputs.addAll(objectService.getObjects(type));
return new ArrayList<>(compatibleInputs);
// Here we compare with object service's items and de-duplicate
Collection<Object> compatibleInputs = convertService.getCompatibleInputs(type);
// NB: we aggressively convert here to avoid listing duplicate items
// that would effectively resolve to the same object
Set<Object> objects = compatibleInputs.stream()
.map(o -> convertService.convert(o, type)).collect(Collectors.toSet());
objects.addAll(objectService.getObjects(type));
return new ArrayList<>(objects);
}
}
29 changes: 1 addition & 28 deletions src/main/java/org/scijava/widget/DefaultWidgetModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@
package org.scijava.widget;

import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.WeakHashMap;

import org.scijava.AbstractContextual;
import org.scijava.Context;
Expand Down Expand Up @@ -60,7 +58,6 @@ public class DefaultWidgetModel extends AbstractContextual implements WidgetMode
private final Module module;
private final ModuleItem<?> item;
private final List<?> objectPool;
private final Map<Object, Object> convertedObjects;

@Parameter
private ThreadService threadService;
Expand All @@ -87,7 +84,6 @@ public DefaultWidgetModel(final Context context, final InputPanel<?, ?> inputPan
this.module = module;
this.item = item;
this.objectPool = objectPool;
convertedObjects = new WeakHashMap<>();

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

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

// Check if a converted value is present
Object convertedInput = convertedObjects.get(value);
if (convertedInput != null &&
Objects.equals(item.getValue(module), convertedInput))
{
return; // no change
}

// Pass the value through the convertService
convertedInput = convertService.convert(value, item.getType());
if (convertedInput == null) convertedInput = value;

// If we get a different (converted) value back, cache it weakly.
if (convertedInput != value) {
convertedObjects.put(value, convertedInput);
}

module.setInput(name, convertedInput);
module.setInput(item.getName(), value);

if (initialized) {
threadService.queue(() -> {
Expand Down Expand Up @@ -309,11 +287,6 @@ private Object ensureValidObject(final Object value) {
private Object ensureValid(final Object value, final List<?> list) {
for (final Object o : list) {
if (o.equals(value)) return value; // value is valid
// check if value was converted and cached
final Object convertedValue = convertedObjects.get(o);
if (convertedValue != null && value.equals(convertedValue)) {
return convertedValue;
}
}

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

0 comments on commit f5b020d

Please sign in to comment.