Skip to content

Add named object index and improve ObjectService #374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jan 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 3 additions & 28 deletions src/main/java/org/scijava/object/DefaultObjectService.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,10 @@

package org.scijava.object;

import java.util.List;

import org.scijava.event.EventHandler;
import org.scijava.event.EventService;
import org.scijava.object.event.ObjectCreatedEvent;
import org.scijava.object.event.ObjectDeletedEvent;
import org.scijava.object.event.ObjectsAddedEvent;
import org.scijava.object.event.ObjectsRemovedEvent;
import org.scijava.plugin.Parameter;
import org.scijava.plugin.Plugin;
import org.scijava.service.AbstractService;
Expand Down Expand Up @@ -68,7 +64,7 @@ public final class DefaultObjectService extends AbstractService implements
private EventService eventService;

/** Index of registered objects. */
private ObjectIndex<Object> objectIndex;
private NamedObjectIndex<Object> objectIndex;

// -- ObjectService methods --

Expand All @@ -78,35 +74,15 @@ public EventService eventService() {
}

@Override
public ObjectIndex<Object> getIndex() {
public NamedObjectIndex<Object> getIndex() {
return objectIndex;
}

@Override
public <T> List<T> getObjects(final Class<T> type) {
final List<Object> list = objectIndex.get(type);
@SuppressWarnings("unchecked")
final List<T> result = (List<T>) list;
return result;
}

@Override
public void addObject(final Object obj) {
objectIndex.add(obj);
eventService.publish(new ObjectsAddedEvent(obj));
}

@Override
public void removeObject(final Object obj) {
objectIndex.remove(obj);
eventService.publish(new ObjectsRemovedEvent(obj));
}

// -- Service methods --

@Override
public void initialize() {
objectIndex = new ObjectIndex<>(Object.class);
objectIndex = new NamedObjectIndex<>(Object.class);
}

// -- Event handlers --
Expand All @@ -120,5 +96,4 @@ protected void onEvent(final ObjectCreatedEvent event) {
protected void onEvent(final ObjectDeletedEvent event) {
removeObject(event.getObject());
}

}
34 changes: 34 additions & 0 deletions src/main/java/org/scijava/object/NamedObjectIndex.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package org.scijava.object;

import java.util.WeakHashMap;

/**
* An {@link ObjectIndex} where each object can have an associated name.
*
* @author Jan Eglinger
*/
public class NamedObjectIndex<E> extends ObjectIndex<E> {

private WeakHashMap<Object, String> nameMap;

public NamedObjectIndex(final Class<E> baseClass) {
super(baseClass);
nameMap = new WeakHashMap<>();
}

public boolean add(E object, String name) {
if (name != null)
nameMap.put(object, name);
return add(object);
}

public boolean add(E object, Class<?> type, String name, boolean batch) {
if (name != null)
nameMap.put(object, name);
return add(object, type, batch);
}

public String getName(E object) {
return nameMap.get(object);
}
}
49 changes: 45 additions & 4 deletions src/main/java/org/scijava/object/ObjectService.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@

import java.util.List;

import org.scijava.Named;
import org.scijava.event.EventService;
import org.scijava.object.event.ObjectsAddedEvent;
import org.scijava.object.event.ObjectsRemovedEvent;
import org.scijava.service.SciJavaService;

/**
Expand All @@ -49,16 +52,54 @@ default EventService eventService() {
}

/** Gets the index of available objects. */
ObjectIndex<Object> getIndex();
NamedObjectIndex<Object> getIndex();

/** Gets a list of all registered objects compatible with the given type. */
<T> List<T> getObjects(Class<T> type);
default <T> List<T> getObjects(final Class<T> type) {
final List<Object> list = getIndex().get(type);
@SuppressWarnings("unchecked")
final List<T> result = (List<T>) list;
return result;
}

/**
* Gets the name belonging to a given object.
* <p>
* If no explicit name was provided at registration time, the name will be
* derived from {@link Named#getName()} if the object implements
* {@link Named}, or from the {@link Object#toString()} otherwise. It is
* guaranteed that this method will not return {@code null}.
* </p>
**/
default String getName(final Object obj) {
if (obj == null) throw new NullPointerException();
final String name = getIndex().getName(obj);
if (name != null) return name;
if (obj instanceof Named) {
final String n = ((Named) obj).getName();
if (n != null) return n;
}
final String s = obj.toString();
if (s != null) return s;
return obj.getClass().getName() + "@" + Integer.toHexString(obj.hashCode());
}

/** Registers an object with the object service. */
void addObject(Object obj);
default void addObject(Object obj) {
addObject(obj, null);
}

/** Registers a named object with the object service. */
default void addObject(final Object obj, final String name) {
getIndex().add(obj, name);
eventService().publish(new ObjectsAddedEvent(obj));
}

/** Deregisters an object with the object service. */
void removeObject(Object obj);
default void removeObject(final Object obj) {
getIndex().remove(obj);
eventService().publish(new ObjectsRemovedEvent(obj));
}

// -- Deprecated methods --

Expand Down
6 changes: 5 additions & 1 deletion src/main/java/org/scijava/widget/DefaultWidgetModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.scijava.module.Module;
import org.scijava.module.ModuleItem;
import org.scijava.module.ModuleService;
import org.scijava.object.ObjectService;
import org.scijava.plugin.Parameter;
import org.scijava.thread.ThreadService;
import org.scijava.util.NumberUtils;
Expand Down Expand Up @@ -74,6 +75,9 @@ public class DefaultWidgetModel extends AbstractContextual implements WidgetMode
@Parameter
private ModuleService moduleService;

@Parameter
private ObjectService objectService;

@Parameter(required = false)
private LogService log;

Expand Down Expand Up @@ -227,7 +231,7 @@ public String[] getChoices() {
final List<?> choicesList = item.getChoices();
final String[] choices = new String[choicesList.size()];
for (int i = 0; i < choices.length; i++) {
choices[i] = choicesList.get(i).toString();
choices[i] = objectService.getName(choicesList.get(i));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change to DefaultWidgetModel is probably superfluous. I figured that dropdown choices such as for images and other objects will be rendered by ObjectWidget and require changing the renderer for list cells as done in scijava/scijava-ui-swing@2411016.

The getChoicesmethod of WidgetModel seems to be exclusively used for the choices attribute of Parameters, right? What confused me was the generic typing List<?> of item.getChoices().

In summary, I am not sure whether we should change this line to use ObjectService, or leave it unchanged.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intuitively, using the ObjectService seems less hacky, so let's change it for now, and revert if any problems arise.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Judging from:

@Override
public List<T> getChoices() {
final T[] choices = getType().getEnumConstants();
return choices == null ? null : Arrays.asList(choices);
}
)

... the ModuleItem#getChoices() seems to be used for enums, other than for String choices.

But I didn't find an actual application case where the choice widget is used when and enum parameter is requested. Is there any?

In any case, the above line will probably never be called on Named objects or on objects registered to the ObjectService, so I'll remove it in a follow-up commit.

}
return choices;
}
Expand Down
43 changes: 43 additions & 0 deletions src/test/java/org/scijava/object/NamedObjectIndexTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package org.scijava.object;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import org.junit.Test;

public class NamedObjectIndexTest {

@Test
public void testNamedObjects() {
NamedObjectIndex<String> index = new NamedObjectIndex<>(String.class);
String obj1 = "obj1";
String name1 = "name1";
String obj2 = "obj1";
String name2 = "name1";
assertTrue(index.add(obj1, name1));
assertTrue(index.add(obj2, String.class, name2, false));
assertTrue(index.contains(obj1));
assertTrue(index.contains(obj2));
assertEquals(name1, index.getName(obj1));
assertEquals(name2, index.getName(obj2));
assertTrue(index.remove(obj1));
assertTrue(index.remove(obj2));
assertFalse(index.contains(obj1));
assertFalse(index.contains(obj2));
}

@Test
public void testNullNames() {
NamedObjectIndex<String> index = new NamedObjectIndex<>(String.class);
String obj1 = "object1";
String name1 = null;
String obj2 = "object2";
String name2 = "";
assertTrue(index.add(obj1, name1));
assertTrue(index.add(obj2, name2));
assertEquals(name1, index.getName(obj1));
assertEquals(name2, index.getName(obj2));
}
}
75 changes: 75 additions & 0 deletions src/test/java/org/scijava/object/ObjectServiceTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package org.scijava.object;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.scijava.Context;
import org.scijava.plugin.PluginInfo;
import org.scijava.plugin.SciJavaPlugin;

public class ObjectServiceTest {

private Context context;
private ObjectService objectService;

@Before
public void setUp() {
context = new Context(ObjectService.class);
objectService = context.getService(ObjectService.class);
}

@After
public void tearDown() {
context.dispose();
}

@Test
public void testAddRemoveObjects() {
Object obj1 = new Object();
String name1 = "Object 1";
Object obj2 = "";
Object obj3 = new Double(0.3);
PluginInfo<SciJavaPlugin> obj4 = PluginInfo.create(TestPlugin.class, SciJavaPlugin.class);
obj4.setName("TestPlugin name");

objectService.addObject(obj1, name1);
assertEquals("Name of object 1", name1, objectService.getName(obj1));
objectService.addObject(obj2);
assertEquals("Name of object 2", obj2.toString(), objectService.getName(obj2));
objectService.addObject(obj3, null);
assertEquals("Name of object 3", obj3.toString(), objectService.getName(obj3));
objectService.addObject(obj4);
assertNotNull(objectService.getName(obj4));
assertEquals("Name of object 4", obj4.getName(), objectService.getName(obj4));

assertTrue("Object 1 registered", objectService.getObjects(Object.class).contains(obj1));
assertTrue("Object 2 registered", objectService.getObjects(Object.class).contains(obj2));
assertTrue("Object 3 registered", objectService.getObjects(Object.class).contains(obj3));
assertTrue("Object 4 registered", objectService.getObjects(Object.class).contains(obj4));

objectService.removeObject(obj1);
objectService.removeObject(obj2);
objectService.removeObject(obj3);
objectService.removeObject(obj4);

assertFalse("Object 1 removed", objectService.getObjects(Object.class).contains(obj1));
assertFalse("Object 2 removed", objectService.getObjects(Object.class).contains(obj2));
assertFalse("Object 3 removed", objectService.getObjects(Object.class).contains(obj3));
assertFalse("Object 4 removed", objectService.getObjects(Object.class).contains(obj4));
}

@Test
public void testNamedObjectIndex() {
ObjectIndex<Object> index = objectService.getIndex();
assertTrue(index instanceof NamedObjectIndex);
}

private class TestPlugin implements SciJavaPlugin {

}
}