Skip to content

Commit 589c92b

Browse files
ctruedenctrueden
committed
Merge branch 'conversion-fixes'
This addresses bugs in SJC's convert subsystem, and improves conversion around generics-aware types.
2 parents fae4829 + 687bd28 commit 589c92b

25 files changed

+700
-692
lines changed

‎pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
</parent>
1111

1212
<artifactId>scijava-common</artifactId>
13-
<version>2.90.3-SNAPSHOT</version>
13+
<version>2.91.0-SNAPSHOT</version>
1414

1515
<name>SciJava Common</name>
1616
<description>SciJava Common is a shared library for SciJava software. It provides a plugin framework, with an extensible mechanism for service discovery, backed by its own annotation processor, so that plugins can be loaded dynamically. It is used by downstream projects in the SciJava ecosystem, such as ImageJ and SCIFIO.</description>

‎src/main/java/org/scijava/convert/AbstractConvertService.java‎

Lines changed: 1 addition & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,6 @@
2929

3030
package org.scijava.convert;
3131

32-
import java.lang.reflect.Type;
33-
import java.util.Collection;
34-
import java.util.HashSet;
35-
import java.util.LinkedHashSet;
36-
import java.util.Set;
37-
3832
import org.scijava.plugin.AbstractHandlerService;
3933

4034
/**
@@ -46,127 +40,5 @@ public abstract class AbstractConvertService //
4640
extends AbstractHandlerService<ConversionRequest, Converter<?, ?>> //
4741
implements ConvertService
4842
{
49-
50-
// -- ConversionService methods --
51-
@SuppressWarnings({ "unchecked", "rawtypes" })
52-
@Override
53-
public Class<Converter<?, ?>> getPluginType() {
54-
return (Class) Converter.class;
55-
}
56-
57-
@Override
58-
public Class<ConversionRequest> getType() {
59-
return ConversionRequest.class;
60-
}
61-
62-
@Override
63-
public Converter<?, ?> getHandler(final Object src, final Class<?> dest) {
64-
return getHandler(new ConversionRequest(src, dest));
65-
}
66-
67-
@Override
68-
public Converter<?, ?> getHandler(final Class<?> src, final Class<?> dest) {
69-
return getHandler(new ConversionRequest(src, dest));
70-
}
71-
72-
@Override
73-
public Converter<?, ?> getHandler(final Object src, final Type dest) {
74-
return getHandler(new ConversionRequest(src, dest));
75-
}
76-
77-
@Override
78-
public Converter<?, ?> getHandler(final Class<?> src, final Type dest) {
79-
return getHandler(new ConversionRequest(src, dest));
80-
}
81-
82-
@Override
83-
public boolean supports(final Object src, final Class<?> dest) {
84-
return supports(new ConversionRequest(src, dest));
85-
}
86-
87-
@Override
88-
public boolean supports(final Class<?> src, final Class<?> dest) {
89-
return supports(new ConversionRequest(src, dest));
90-
}
91-
92-
@Override
93-
public boolean supports(final Object src, final Type dest) {
94-
return supports(new ConversionRequest(src, dest));
95-
}
96-
97-
@Override
98-
public boolean supports(final Class<?> src, final Type dest) {
99-
return supports(new ConversionRequest(src, dest));
100-
}
101-
102-
@Override
103-
public Collection<Object> getCompatibleInputs(final Class<?> dest) {
104-
final Set<Object> objects = new LinkedHashSet<>();
105-
106-
for (final Converter<?, ?> c : getInstances()) {
107-
if (dest.isAssignableFrom(c.getOutputType())) {
108-
c.populateInputCandidates(objects);
109-
}
110-
}
111-
112-
return objects;
113-
}
114-
115-
@Override
116-
public Object convert(final Object src, final Type dest) {
117-
return convert(new ConversionRequest(src, dest));
118-
}
119-
120-
@Override
121-
public <T> T convert(final Object src, final Class<T> dest) {
122-
// NB: repeated code with convert(ConversionRequest), because the
123-
// handler's convert method respects the T provided
124-
final Converter<?, ?> handler = getHandler(src, dest);
125-
return handler == null ? null : handler.convert(src, dest);
126-
}
127-
128-
@Override
129-
public Object convert(final ConversionRequest request) {
130-
final Converter<?, ?> handler = getHandler(request);
131-
return handler == null ? null : handler.convert(request);
132-
}
133-
134-
@Override
135-
public Collection<Class<?>> getCompatibleInputClasses(final Class<?> dest) {
136-
final Set<Class<?>> compatibleClasses = new HashSet<>();
137-
138-
for (final Converter<?, ?> converter : getInstances()) {
139-
addIfMatches(dest, converter.getOutputType(), converter.getInputType(), compatibleClasses);
140-
}
141-
142-
return compatibleClasses;
143-
}
144-
145-
@Override
146-
public Collection<Class<?>> getCompatibleOutputClasses(final Class<?> source) {
147-
final Set<Class<?>> compatibleClasses = new HashSet<>();
148-
149-
for (final Converter<?, ?> converter : getInstances()) {
150-
try {
151-
addIfMatches(source, converter.getInputType(), converter.getOutputType(), compatibleClasses);
152-
}
153-
catch (final Throwable t) {
154-
log().error("Malfunctioning converter plugin: " + //
155-
converter.getClass().getName(), t);
156-
}
157-
}
158-
159-
return compatibleClasses;
160-
}
161-
162-
// -- Helper methods --
163-
164-
/**
165-
* Test two classes; if they match, a third class is added to the provided
166-
* set of classes.
167-
*/
168-
private void addIfMatches(final Class<?> c1, final Class<?> c2, final Class<?> toAdd, final Set<Class<?>> classes) {
169-
if (c1 == c2)
170-
classes.add(toAdd);
171-
}
43+
// NB: This layer remains merely for backwards compatibility.
17244
}

‎src/main/java/org/scijava/convert/AbstractConverter.java‎

Lines changed: 4 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -29,37 +29,17 @@
2929

3030
package org.scijava.convert;
3131

32-
import java.lang.reflect.Type;
3332
import java.util.Collection;
3433

3534
import org.scijava.object.ObjectService;
3635
import org.scijava.plugin.AbstractHandlerPlugin;
3736
import org.scijava.plugin.Parameter;
38-
import org.scijava.util.Types;
3937

4038
/**
4139
* Abstract superclass for {@link Converter} plugins. Performs appropriate
4240
* dispatching of {@link #canConvert(ConversionRequest)} and
4341
* {@link #convert(ConversionRequest)} calls based on the actual state of the
4442
* given {@link ConversionRequest}.
45-
* <p>
46-
* Note that the {@link #supports(ConversionRequest)} method is overridden as
47-
* well, to delegate to the appropriate {@link #canConvert}.
48-
* </p>
49-
* <p>
50-
* NB: by default, the {@link #populateInputCandidates(Collection)} method has a
51-
* dummy implementation. Effectively, this is opt-in behavior. If a converter
52-
* implementation would like to suggest candidates for conversion, this method
53-
* can be overridden.
54-
* </p>
55-
* <p>
56-
* NB: by default, the provied {@link #canConvert} methods will return
57-
* {@code false} if the input is {@code null}. This allows {@link Converter}
58-
* implementors to assume any input is non-{@code null} - but this behavior is
59-
* overridden. Casting {@code null Object} inputs is handled by the
60-
* {@link NullConverter}, while {@code null class} inputs are handled by the
61-
* {@link DefaultConverter}.
62-
* </p>
6343
*
6444
* @author Mark Hiner
6545
*/
@@ -72,59 +52,7 @@ public abstract class AbstractConverter<I, O> extends
7252
@Parameter(required = false)
7353
private ObjectService objectService;
7454

75-
// -- ConversionHandler methods --
76-
77-
@Override
78-
public boolean canConvert(final ConversionRequest request) {
79-
Object src = request.sourceObject();
80-
if (src == null) {
81-
Class<?> srcClass = request.sourceClass();
82-
if (request.destType() != null) return canConvert(srcClass, request.destType());
83-
return canConvert(srcClass, request.destClass());
84-
}
85-
86-
if (request.destType() != null) return canConvert(src, request.destType());
87-
return canConvert(src, request.destClass());
88-
}
89-
90-
@Override
91-
public boolean canConvert(final Object src, final Type dest) {
92-
if (src == null) return false;
93-
final Class<?> srcClass = src.getClass();
94-
return canConvert(srcClass, dest);
95-
}
96-
97-
@Override
98-
public boolean canConvert(final Object src, final Class<?> dest) {
99-
if (src == null) return false;
100-
final Class<?> srcClass = src.getClass();
101-
102-
return canConvert(srcClass, dest);
103-
}
104-
105-
@Override
106-
public boolean canConvert(final Class<?> src, final Class<?> dest) {
107-
if (src == null) return false;
108-
final Class<?> saneSrc = Types.box(src);
109-
final Class<?> saneDest = Types.box(dest);
110-
return Types.isAssignable(saneSrc, getInputType()) &&
111-
Types.isAssignable(getOutputType(), saneDest);
112-
}
113-
114-
@Override
115-
public Object convert(final Object src, final Type dest) {
116-
final Class<?> destClass = Types.raw(dest);
117-
return convert(src, destClass);
118-
}
119-
120-
@Override
121-
public Object convert(final ConversionRequest request) {
122-
if (request.destType() != null) {
123-
return convert(request.sourceObject(), request.destType());
124-
}
125-
126-
return convert(request.sourceObject(), request.destClass());
127-
}
55+
// -- Converter methods --
12856

12957
@Override
13058
public void populateInputCandidates(final Collection<Object> objects) {
@@ -138,20 +66,8 @@ public void populateInputCandidates(final Collection<Object> objects) {
13866

13967
@Override
14068
public boolean supports(final ConversionRequest request) {
141-
return canConvert(request);
142-
}
143-
144-
@Override
145-
public Class<ConversionRequest> getType() {
146-
return ConversionRequest.class;
147-
}
148-
149-
// -- Deprecated API --
150-
151-
@Override
152-
@Deprecated
153-
public boolean canConvert(final Class<?> src, final Type dest) {
154-
final Class<?> destClass = Types.raw(dest);
155-
return canConvert(src, destClass);
69+
// NB: Overridden just for backwards compatibility, so that
70+
// downstream classes which call super.supports do the right thing.
71+
return Converter.super.supports(request);
15672
}
15773
}

‎src/main/java/org/scijava/convert/ArrayToStringConverter.java‎

Lines changed: 33 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,11 @@
3737
import org.scijava.plugin.Parameter;
3838
import org.scijava.plugin.Plugin;
3939
import org.scijava.util.ArrayUtils;
40-
import org.scijava.util.Types;
4140

4241
/**
43-
* A {@link Converter} that specializes in converting
44-
* n-dimensional arrays into {@link String}s. This {@link Converter} can convert any array whose
45-
* component types can be converted into {@link String}s. By default, this
42+
* A {@link Converter} that specializes in converting n-dimensional arrays into
43+
* {@link String}s. This {@link Converter} can convert any array whose component
44+
* types can be converted into {@link String}s. By default, this
4645
* {@link Converter} delimits the array elements with commas.
4746
*
4847
* @author Gabriel Selzer
@@ -53,77 +52,77 @@ public class ArrayToStringConverter extends AbstractConverter<Object, String> {
5352
@Parameter(required = false)
5453
private ConvertService convertService;
5554

56-
@Override public boolean canConvert(final Class<?> src, final Class<?> dest) {
57-
if (src == null) return false;
58-
final Class<?> saneSrc = Types.box(src);
59-
final Class<?> saneDest = Types.box(dest);
60-
return saneSrc.isArray() && saneDest == String.class;
55+
@Override
56+
public boolean canConvert(final Class<?> src, final Class<?> dest) {
57+
return src != null && src.isArray() && dest == String.class;
6158
}
6259

63-
@Override public boolean canConvert(final Object src, final Class<?> dest) {
64-
if (convertService == null) return false;
60+
@Override
61+
public boolean canConvert(final Object src, final Class<?> dest) {
62+
if (convertService == null || src == null) return false;
6563
if (!canConvert(src.getClass(), dest)) return false;
6664
if (Array.getLength(src) == 0) return true;
6765
return convertService.supports(Array.get(src, 0), dest);
6866
}
6967

7068
@Override
71-
public Object convert(Object src, Type dest) {
69+
public Object convert(Object src, final Type dest) {
7270
// Preprocess the "string-likes"
73-
if (src.getClass().equals(String[].class) || //
74-
src.getClass().equals(Character[].class) || //
75-
src.getClass().equals(char[].class)) //
71+
final Class<?> srcClass = src.getClass();
72+
if (srcClass == String[].class || //
73+
srcClass == Character[].class || //
74+
srcClass == char[].class) //
7675
{
7776
src = preprocessCharacters(src);
7877
}
7978
// Convert each element to Strings
80-
String elementString = ArrayUtils.toCollection(src).stream() //
79+
final String elementString = ArrayUtils.toCollection(src).stream() //
8180
.map(object -> convertService.convert(object, String.class)) //
8281
.collect(Collectors.joining(", "));
8382
return "{" + elementString + "}";
8483
}
8584

8685
private String[] preprocessStrings(final Object src) {
87-
int numElements = Array.getLength(src);
88-
String[] processed = new String[numElements];
86+
final int numElements = Array.getLength(src);
87+
final String[] processed = new String[numElements];
8988
for (int i = 0; i < numElements; i++) {
9089
processed[i] = preprocessString(Array.get(src, i));
9190
}
9291
return processed;
9392
}
9493

9594
private String preprocessString(final Object o) {
96-
String s;
97-
if (o == null) {
98-
return null;
99-
} else {
100-
s = o.toString();
101-
s = s.replace("\\", "\\\\");
102-
s = s.replace("\"", "\\\"");
103-
}
95+
if (o == null) return null;
96+
String s = o.toString();
97+
s = s.replace("\\", "\\\\");
98+
s = s.replace("\"", "\\\"");
10499
return "\"" + s + "\"";
105100
}
106101

107102
private String[] preprocessCharacters(Object src) {
108-
String[] processed = new String[Array.getLength(src)];
103+
final String[] processed = new String[Array.getLength(src)];
109104
for (int i = 0; i < processed.length; i++) {
110-
Object value = Array.get(src, i);
105+
final Object value = Array.get(src, i);
111106
processed[i] = value == null ? null : value.toString();
112107
}
113108
return preprocessStrings(processed);
114109
}
115110

116-
@SuppressWarnings("unchecked") @Override
117-
public <T> T convert(Object src, Class<T> dest) {
118-
return (T) convert(src, (Type) dest);
111+
@Override
112+
public <T> T convert(final Object src, final Class<T> dest) {
113+
final Type destType = dest;
114+
@SuppressWarnings("unchecked")
115+
final T converted = (T) convert(src, destType);
116+
return converted;
119117
}
120118

121-
@Override public Class<String> getOutputType() {
119+
@Override
120+
public Class<String> getOutputType() {
122121
return String.class;
123122
}
124123

125-
@Override public Class<Object> getInputType() {
124+
@Override
125+
public Class<Object> getInputType() {
126126
return Object.class;
127127
}
128-
129128
}

0 commit comments

Comments
 (0)