Skip to content

Commit 9fc370e

Browse files
wiedenmctrueden
wiedenm
authored and
ctrueden
committed
Resolve Op dependencies before constructing Op
And use them for construction. This commit should resolve #16.
1 parent 3f839dc commit 9fc370e

6 files changed

Lines changed: 71 additions & 47 deletions

File tree

‎src/main/java/org/scijava/ops/OpService.java‎

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -188,48 +188,35 @@ public LogService logger() {
188188
* matching the functional type and the name could not be found, if
189189
* an exception occurs during injection
190190
*/
191-
private void resolveOpDependencies(Object op, OpCandidate parentOp) throws OpMatchingException {
192-
// HACK: Only works with Op instances and OpRunner, not extensible.
193-
// Consider extensible ways to achieve something similar e.g. extending OpInfo
194-
// to support OpDependencies.
195-
if (op instanceof OpRunner) {
196-
op = ((OpRunner<?>) op).getAdaptedOp();
197-
}
198-
final List<OpDependencyMember<?>> dependencies = parentOp.opInfo()
199-
.dependencies();
191+
private List<Object> resolveOpDependencies(OpCandidate op)
192+
throws OpMatchingException
193+
{
194+
final List<OpDependencyMember<?>> dependencies = op.opInfo().dependencies();
195+
final List<Object> resolvedDependencies = new ArrayList<>(dependencies
196+
.size());
200197
for (final OpDependencyMember<?> dependency : dependencies) {
201198
final String dependencyName = dependency.getDependencyName();
202199
final Type mappedDependencyType = Types.mapVarToTypes(new Type[] {
203-
dependency.getType() }, parentOp.typeVarAssigns())[0];
200+
dependency.getType() }, op.typeVarAssigns())[0];
204201
final OpRef inferredRef = inferOpRef(mappedDependencyType, dependencyName,
205-
parentOp.typeVarAssigns());
202+
op.typeVarAssigns());
206203
if (inferredRef == null) {
207204
throw new OpMatchingException("Could not infer functional " +
208205
"method inputs and outputs of Op dependency field: " + dependency
209206
.getKey());
210207
}
211-
Object matchedOp = null;
212208
try {
213-
matchedOp = findOpInstance(dependencyName, inferredRef);
209+
resolvedDependencies.add(findOpInstance(dependencyName, inferredRef));
214210
}
215211
catch (final Exception e) {
216212
throw new OpMatchingException(
217-
"Could not find Op that matches requested Op dependency field:" +
218-
"\nOp class: " + op.getClass().getName() + //
219-
"\nDependency field: " + dependency.getKey() + //
213+
"Could not find Op that matches requested Op dependency:" +
214+
"\nOp class: " + op.opInfo().implementationName() + //
215+
"\nDependency identifier: " + dependency.getKey() + //
220216
"\n\n Attempted request:\n" + inferredRef, e);
221217
}
222-
try {
223-
dependency.createInstance(op).set(matchedOp);
224-
}
225-
catch (final Exception e) {
226-
throw new OpMatchingException(
227-
"Exception trying to inject Op dependency field.\n" +
228-
"\tOp dependency field to resolve: " + dependency.getKey() + "\n" +
229-
"\tFound Op to inject: " + matchedOp.getClass().getName() + "\n" +
230-
"\tWith inferred OpRef: " + inferredRef, e);
231-
}
232218
}
219+
return resolvedDependencies;
233220
}
234221

235222
@SuppressWarnings("unchecked")
@@ -247,7 +234,8 @@ public Object findOpInstance(final String opName, final OpRef ref, final Object.
247234
try {
248235
// Find single match which matches the specified types
249236
match = matcher.findSingleMatch(this, ref);
250-
op = match.createOp(secondaryArgs);
237+
final List<Object> dependencies = resolveOpDependencies(match);
238+
op = match.createOp(dependencies, secondaryArgs);
251239
} catch (OpMatchingException e) {
252240
log.debug("No matching Op for request: " + ref + "\n");
253241
log.debug("Attempting Op transformation...");
@@ -263,17 +251,19 @@ public Object findOpInstance(final String opName, final OpRef ref, final Object.
263251
// If we found one, try to do transformation and return transformed op
264252
log.debug("Matching Op transformation found:\n" + transformation + "\n");
265253
try {
266-
op = transformation.exceute(this, secondaryArgs);
254+
final List<Object> dependencies = resolveOpDependencies(transformation
255+
.getSourceOp());
256+
op = transformation.exceute(this, dependencies, secondaryArgs);
267257
} catch (OpMatchingException | OpTransformationException e1) {
268258
throw new IllegalArgumentException("Execution of Op transformatioon failed:\n" + e1);
269259
}
270260
}
271261
try {
272262
// Try to resolve annotated OpDependency fields
273263
if (match != null)
274-
resolveOpDependencies(op, match);
264+
resolveOpDependencies(match);
275265
else if (transformation != null)
276-
resolveOpDependencies(op, transformation.getSourceOp());
266+
resolveOpDependencies(transformation.getSourceOp());
277267
} catch (OpMatchingException e) {
278268
throw new IllegalArgumentException(e);
279269
}

‎src/main/java/org/scijava/ops/matcher/OpCandidate.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131

3232
import java.lang.reflect.Type;
3333
import java.lang.reflect.TypeVariable;
34+
import java.util.List;
3435
import java.util.Map;
3536

3637
import org.scijava.ops.OpEnvironment;
@@ -202,19 +203,22 @@ public String toString() {
202203
return info.toString();
203204
}
204205

205-
public StructInstance<?> createOpInstance(Object... secondaryArgs) throws OpMatchingException {
206+
public StructInstance<?> createOpInstance(List<?> dependencies, Object... secondaryArgs) throws OpMatchingException
207+
{
206208
if (!getStatusCode().equals(StatusCode.MATCH)) {
207209
throw new OpMatchingException(
208210
"Status of candidate to create op from indicates a problem: " + getStatus());
209211
}
210212

211-
StructInstance<?> inst = opInfo().createOpInstance();
213+
StructInstance<?> inst = opInfo().createOpInstance(dependencies);
212214
inject(inst, secondaryArgs);
213215
return inst;
214216
}
215217

216-
public Object createOp(Object... secondaryArgs) throws OpMatchingException {
217-
return createOpInstance(secondaryArgs).object();
218+
public Object createOp(List<?> dependencies, Object... secondaryArgs)
219+
throws OpMatchingException
220+
{
221+
return createOpInstance(dependencies, secondaryArgs).object();
218222
}
219223

220224
private void inject(StructInstance<?> opInst, Object... secondaryArgs) {

‎src/main/java/org/scijava/ops/matcher/OpClassInfo.java‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@
3232
import java.lang.reflect.Constructor;
3333
import java.lang.reflect.InvocationTargetException;
3434
import java.lang.reflect.Type;
35+
import java.util.List;
3536

3637
import org.scijava.core.Priority;
38+
import org.scijava.ops.OpDependencyMember;
3739
import org.scijava.ops.OpUtils;
3840
import org.scijava.ops.core.Op;
3941
import org.scijava.param.ParameterStructs;
@@ -91,22 +93,43 @@ public String implementationName() {
9193
}
9294

9395
@Override
94-
public StructInstance<?> createOpInstance() {
95-
final Object object;
96+
public StructInstance<?> createOpInstance(List<?> dependencies) {
97+
final Object op;
9698
try {
9799
// TODO: Consider whether this is really the best way to
98100
// instantiate the op class here. No framework usage?
99101
// E.g., what about pluginService.createInstance?
100102
Constructor<? extends Op> ctor = opClass.getDeclaredConstructor();
101103
ctor.setAccessible(true);
102-
object = ctor.newInstance();
103-
} catch (final InstantiationException | IllegalAccessException | NoSuchMethodException | SecurityException | IllegalArgumentException | InvocationTargetException e) {
104+
op = ctor.newInstance();
105+
}
106+
catch (final InstantiationException | IllegalAccessException
107+
| NoSuchMethodException | SecurityException | IllegalArgumentException
108+
| InvocationTargetException e)
109+
{
104110
// TODO: Think about whether exception handling here should be
105111
// different.
106-
throw new IllegalStateException("Unable to instantiate op: '" + opClass.getName()
107-
+ "' Ensure that the Op has a no-args constructor.", e);
112+
throw new IllegalStateException("Unable to instantiate op: '" + opClass
113+
.getName() + "' Ensure that the Op has a no-args constructor.", e);
114+
}
115+
final List<OpDependencyMember<?>> dependencyMembers = dependencies();
116+
for (int i = 0; i < dependencyMembers.size(); i++) {
117+
final OpDependencyMember<?> dependencyMember = dependencyMembers.get(i);
118+
try {
119+
dependencyMember.createInstance(op).set(dependencies.get(i));
120+
}
121+
catch (final Exception ex) {
122+
// TODO: Improve error message. Used to include exact OpRef of Op
123+
// dependency.
124+
throw new IllegalStateException(
125+
"Exception trying to inject Op dependency field.\n" +
126+
"\tOp dependency field to resolve: " + dependencyMember.getKey() +
127+
"\n" + "\tFound Op to inject: " + dependencies.get(i).getClass()
128+
.getName() + //
129+
"\n" + "\tField signature: " + dependencyMember.getType(), ex);
130+
}
108131
}
109-
return struct().createInstance(object);
132+
return struct().createInstance(op);
110133
}
111134

112135
@Override

‎src/main/java/org/scijava/ops/matcher/OpFieldInfo.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,11 @@ public String implementationName() {
110110
}
111111

112112
@Override
113-
public StructInstance<?> createOpInstance() {
113+
public StructInstance<?> createOpInstance(List<?> dependencies)
114+
{
115+
if (dependencies != null && !dependencies.isEmpty())
116+
throw new IllegalArgumentException(
117+
"Op fields are not allowed to have any Op dependencies.");
114118
// 1. Can we create another instance of the same function by calling
115119
// clone()?
116120
// 2. _SHOULD_ we do that? Or can we simply reuse the same function

‎src/main/java/org/scijava/ops/matcher/OpInfo.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ default List<OpDependencyMember<?>> dependencies() {
4646
/** A fully qualified, unambiguous name for this specific op implementation. */
4747
String implementationName();
4848

49-
/** Create a StructInstance using the Struct metadata backed by an object of the op itself. */
50-
StructInstance<?> createOpInstance();
49+
/** Create a StructInstance using the Struct metadata backed by an object of the op itself. */
50+
StructInstance<?> createOpInstance(List<?> dependencies);
5151

5252
// TODO Consider if we really want to keep the following methods.
5353
boolean isValid();

‎src/main/java/org/scijava/ops/transform/OpTransformationCandidate.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package org.scijava.ops.transform;
22

3+
import java.util.List;
4+
35
import org.scijava.ops.OpService;
46
import org.scijava.ops.matcher.OpCandidate;
57
import org.scijava.ops.matcher.OpMatchingException;
@@ -24,9 +26,10 @@ public OpCandidate getSourceOp() {
2426
return srcOp;
2527
}
2628

27-
public Object exceute(OpService opService, Object... secondaryArgs)
28-
throws OpMatchingException, OpTransformationException {
29-
Object op = srcOp.createOp(secondaryArgs);
29+
public Object exceute(OpService opService, List<?> dependencies, Object... secondaryArgs)
30+
throws OpMatchingException, OpTransformationException
31+
{
32+
Object op = srcOp.createOp(dependencies, secondaryArgs);
3033
return transformation.execute(op, opService);
3134
}
3235

0 commit comments

Comments
 (0)