Skip to content

Commit 28755f9

Browse files
hinermhinerm
authored
Merge pull request #83 from scijava/scijava/scijava-ops-api/remove-validity-exception
Throw ValidityException instead of saving it
2 parents 3c5f06c + a8a9c07 commit 28755f9

17 files changed

Lines changed: 120 additions & 188 deletions

File tree

‎scijava/scijava-common3/src/main/java/org/scijava/common3/validity/ValidityException.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* @author Curtis Rueden
1111
* @author Gabriel Selzer
1212
*/
13-
public final class ValidityException extends Exception {
13+
public final class ValidityException extends RuntimeException {
1414

1515
private final List<ValidityProblem> problems;
1616

‎scijava/scijava-ops-api/src/main/java/org/scijava/ops/api/OpCandidate.java‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import java.util.List;
3737
import java.util.Map;
3838

39-
import org.scijava.common3.validity.ValidityProblem;
4039
import org.scijava.struct.Member;
4140
import org.scijava.struct.Struct;
4241
import org.scijava.struct.StructInstance;
@@ -53,7 +52,6 @@ public class OpCandidate {
5352

5453
public static enum StatusCode {
5554
MATCH, //
56-
INVALID_STRUCT, //
5755
OUTPUT_TYPES_DO_NOT_MATCH, //
5856
TOO_MANY_ARGS, //
5957
TOO_FEW_ARGS, //
@@ -183,13 +181,6 @@ public String getStatus() {
183181
case MATCH:
184182
sb.append("MATCH");
185183
break;
186-
case INVALID_STRUCT:
187-
sb.append("Invalid struct:");
188-
for (ValidityProblem vp : opInfo().getValidityException().problems()) {
189-
sb.append("\n\t");
190-
sb.append(vp.getMessage());
191-
}
192-
break;
193184
case OUTPUT_TYPES_DO_NOT_MATCH:
194185
sb.append("Output types do not match");
195186
break;

‎scijava/scijava-ops-api/src/main/java/org/scijava/ops/api/OpInfo.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,6 @@ default OpCandidate createCandidate(OpEnvironment env, OpRef ref, Map<TypeVariab
9595
/** Create a StructInstance using the Struct metadata backed by an object of the op itself. */
9696
StructInstance<?> createOpInstance(List<?> dependencies);
9797

98-
// TODO Consider if we really want to keep the following methods.
99-
boolean isValid();
100-
ValidityException getValidityException();
101-
10298
AnnotatedElement getAnnotationBearer();
10399

104100
@Override

‎scijava/scijava-ops-engine/src/main/java/org/scijava/ops/engine/DefaultOpEnvironment.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import java.util.ServiceLoader;
4545
import java.util.Set;
4646
import java.util.function.BiConsumer;
47-
import java.util.function.Function;
4847
import java.util.stream.Collectors;
4948
import java.util.stream.StreamSupport;
5049

@@ -760,11 +759,6 @@ private synchronized void initIdDirectory() {
760759
"Op implementation must provide name.");
761760
return;
762761
}
763-
if (!opInfo.isValid()) {
764-
log.error("Skipping invalid Op " + opInfo.implementationName() + ":\n" +
765-
opInfo.getValidityException().getMessage());
766-
return;
767-
}
768762
for (String opName : opInfo.names()) {
769763
opDirectory.computeIfAbsent(opName, name -> new ArrayList<>()).add(opInfo);
770764
}

‎scijava/scijava-ops-engine/src/main/java/org/scijava/ops/engine/OpUtils.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -111,16 +111,14 @@ public static List<MemberInstance<?>> inputs(StructInstance<?> op) {
111111
.collect(Collectors.toList());
112112
}
113113

114-
public static void checkHasSingleOutput(Struct struct) throws
114+
public static void ensureHasSingleOutput(Struct struct, List<ValidityProblem> problems) throws
115115
ValidityException
116116
{
117117
final long numOutputs = struct.members().stream() //
118-
.filter(m -> m.isOutput()).count();
119-
if (numOutputs != 1) {
120-
final String error = numOutputs == 0 //
121-
? "No output parameters specified. Must specify exactly one." //
122-
: "Multiple output parameters specified. Only a single output is allowed.";
123-
throw new ValidityException(Collections.singletonList(new ValidityProblem(error)));
118+
.filter(Member::isOutput).count();
119+
if (numOutputs > 1) {
120+
problems.add(new ValidityProblem(
121+
"Multiple output parameters specified. Only a single output is allowed."));
124122
}
125123
}
126124

‎scijava/scijava-ops-engine/src/main/java/org/scijava/ops/engine/impl/OpClassOpInfoGenerator.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import java.util.Collections;
55
import java.util.List;
66

7+
import org.scijava.common3.validity.ValidityException;
78
import org.scijava.meta.Versions;
89
import org.scijava.ops.api.*;
910
import org.scijava.ops.engine.OpUtils;
@@ -28,8 +29,13 @@ protected List<OpInfo> processClass(Class<?> c) {
2829
String version = Versions.getVersion(c);
2930
Hints hints = formHints(c.getAnnotation(OpHints.class));
3031
double priority = p.priority();
31-
return Collections.singletonList(new OpClassInfo(c, version, hints,
32-
priority, parsedOpNames));
32+
try {
33+
return Collections.singletonList(
34+
new OpClassInfo(c, version, hints, priority, parsedOpNames));
35+
} catch (ValidityException e) {
36+
// TODO: Log exception
37+
return Collections.emptyList();
38+
}
3339
}
3440

3541
@Override public boolean canGenerateFrom(Object o) {

‎scijava/scijava-ops-engine/src/main/java/org/scijava/ops/engine/impl/OpCollectionInfoGenerator.java‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
import java.lang.reflect.Modifier;
77
import java.util.ArrayList;
88
import java.util.List;
9+
import java.util.Objects;
910
import java.util.Optional;
1011
import java.util.stream.Collectors;
1112

13+
import org.scijava.common3.validity.ValidityException;
1214
import org.scijava.ops.engine.OpUtils;
1315
import org.scijava.common3.Annotations;
1416
import org.scijava.meta.Versions;
@@ -41,14 +43,30 @@ protected List<OpInfo> processClass(Class<?> cls) {
4143
if (instance.isPresent()) {
4244
final List<OpFieldInfo> fieldInfos = //
4345
fields.parallelStream() //
44-
.map(f -> generateFieldInfo(f, instance.get(), version)) //
46+
.map(f -> {
47+
try {
48+
return generateFieldInfo(f, instance.get(), version);
49+
} catch(ValidityException e) {
50+
// TODO: Log exception
51+
return null;
52+
}
53+
}) //
54+
.filter(Objects::nonNull) //
4555
.collect(Collectors.toList());
4656
collectionInfos.addAll(fieldInfos);
4757
}
4858
// add OpMethodInfos
4959
final List<OpMethodInfo> methodInfos = //
5060
Annotations.getAnnotatedMethods(cls, OpMethod.class).parallelStream() //
51-
.map(m -> generateMethodInfo(m, version)) //
61+
.map(m -> {
62+
try {
63+
return generateMethodInfo(m, version);
64+
} catch(ValidityException e) {
65+
// TODO: Log exception
66+
return null;
67+
}
68+
}) //
69+
.filter(Objects::nonNull) //
5270
.collect(Collectors.toList());
5371
collectionInfos.addAll(methodInfos);
5472
return collectionInfos;

‎scijava/scijava-ops-engine/src/main/java/org/scijava/ops/engine/impl/TherapiOpInfoGenerator.java‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.util.ServiceLoader;
1010
import java.util.stream.Collectors;
1111

12+
import org.scijava.common3.validity.ValidityException;
1213
import org.scijava.discovery.therapi.TaggedElement;
1314
import org.scijava.discovery.therapi.TherapiDiscoveryUtils;
1415
import org.scijava.meta.Versions;
@@ -85,7 +86,12 @@ private static String[] getNames(TaggedElement t) {
8586

8687
private static OpInfo opClassGenerator(Class<?> cls, double priority, String[] names) {
8788
String version = Versions.getVersion(cls);
88-
return new OpClassInfo(cls, version, new DefaultHints(), priority, names);
89+
try {
90+
return new OpClassInfo(cls, version, new DefaultHints(), priority, names);
91+
} catch (ValidityException e) {
92+
// TODO: Log exception
93+
return null;
94+
}
8995
}
9096

9197
private static OpInfo opMethodGenerator(Method m, String opType, double priority, String[] names) {
@@ -96,7 +102,12 @@ private static OpInfo opMethodGenerator(Method m, String opType, double priority
96102
return null;
97103
}
98104
String version = Versions.getVersion(m.getDeclaringClass());
99-
return new OpMethodInfo(m, cls, version, new DefaultHints(), priority, names);
105+
try {
106+
return new OpMethodInfo(m, cls, version, new DefaultHints(), priority, names);
107+
} catch (ValidityException e) {
108+
// TODO: Log exception
109+
return null;
110+
}
100111
}
101112

102113
private static OpInfo opFieldGenerator(Field f, double priority, String[] names) {
@@ -108,6 +119,11 @@ private static OpInfo opFieldGenerator(Field f, double priority, String[] names)
108119
| NoSuchMethodException | SecurityException exc) {
109120
return null;
110121
}
111-
return new OpFieldInfo(instance, f, version, new DefaultHints(), priority, names);
122+
try {
123+
return new OpFieldInfo(instance, f, version, new DefaultHints(), priority, names);
124+
} catch (ValidityException e) {
125+
// TODO: Log exception
126+
return null;
127+
}
112128
}
113129
}

‎scijava/scijava-ops-engine/src/main/java/org/scijava/ops/engine/matcher/impl/OpAdaptationInfo.java‎

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -44,33 +44,26 @@ public class OpAdaptationInfo implements OpInfo {
4444
private final Hints hints;
4545

4646
private Struct struct;
47-
private ValidityException validityException;
4847

4948
public OpAdaptationInfo(OpInfo srcInfo, Type type,
5049
InfoChain adaptorChain)
5150
{
5251
this.srcInfo = srcInfo;
5352
this.adaptorChain = adaptorChain;
5453
this.type = type;
54+
this.hints = srcInfo.declaredHints().plus(Adaptation.FORBIDDEN);
5555

5656
// NOTE: since the source Op has already been shown to be valid, there is
57-
// not
58-
// much for us to do here.
57+
// not much for us to do here.
5958
List<ValidityProblem> problems = new ArrayList<>();
6059
List<FunctionalMethodType> fmts = FunctionalParameters.findFunctionalMethodTypes(type);
6160

6261
RetypingRequest r = new RetypingRequest(srcInfo.struct(), fmts);
6362
struct = Structs.from(r, type, problems, new OpRetypingMemberParser());
64-
try {
65-
OpUtils.checkHasSingleOutput(struct);
66-
}
67-
catch (ValidityException exc) {
68-
problems.addAll(exc.problems());
63+
OpUtils.ensureHasSingleOutput(struct, problems);
64+
if (!problems.isEmpty()) {
65+
throw new ValidityException(problems);
6966
}
70-
if (!problems.isEmpty()) validityException = new ValidityException(
71-
problems);
72-
73-
this.hints = srcInfo.declaredHints().plus(Adaptation.FORBIDDEN);
7467
}
7568

7669
@Override
@@ -124,16 +117,6 @@ public StructInstance<?> createOpInstance(List<?> dependencies) {
124117
return struct().createInstance(adaptedOp);
125118
}
126119

127-
@Override
128-
public boolean isValid() {
129-
return validityException == null;
130-
}
131-
132-
@Override
133-
public ValidityException getValidityException() {
134-
return validityException;
135-
}
136-
137120
@Override
138121
public AnnotatedElement getAnnotationBearer() {
139122
return srcInfo.getAnnotationBearer();

‎scijava/scijava-ops-engine/src/main/java/org/scijava/ops/engine/matcher/impl/OpClassInfo.java‎

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ public class OpClassInfo implements OpInfo {
6565
private final Class<?> opClass;
6666
private final String version;
6767
private Struct struct;
68-
private ValidityException validityException;
6968
private final double priority;
7069
private final Hints hints;
7170

@@ -81,16 +80,15 @@ public OpClassInfo(final Class<?> opClass, final String version, final Hints hin
8180
this.opClass = opClass;
8281
this.version = version;
8382
this.names = Arrays.asList(names);
84-
List<ValidityProblem> problems = new ArrayList<>();
85-
try {
86-
struct = Structs.from(opClass, opClass, problems, new ClassParameterMemberParser(), new ClassOpDependencyMemberParser());
87-
OpUtils.checkHasSingleOutput(struct);
88-
} catch (ValidityException e) {
89-
validityException = e;
90-
}
9183
this.priority = priority;
92-
9384
this.hints = hints;
85+
86+
List<ValidityProblem> problems = new ArrayList<>();
87+
struct = Structs.from(opClass, opClass, problems, new ClassParameterMemberParser(), new ClassOpDependencyMemberParser());
88+
OpUtils.ensureHasSingleOutput(struct, problems);
89+
if (!problems.isEmpty()) {
90+
throw new ValidityException(problems);
91+
}
9492
}
9593

9694
// -- OpInfo methods --
@@ -167,16 +165,6 @@ public StructInstance<?> createOpInstance(List<?> dependencies) {
167165
return struct().createInstance(op);
168166
}
169167

170-
@Override
171-
public ValidityException getValidityException() {
172-
return validityException;
173-
}
174-
175-
@Override
176-
public boolean isValid() {
177-
return validityException == null;
178-
}
179-
180168
@Override
181169
public AnnotatedElement getAnnotationBearer() {
182170
return opClass;

0 commit comments

Comments
 (0)