Skip to content

Commit f530f91

Browse files
author
Olha Yelisieieva
committed
Issue-19: Fixed analyzing required properties in the models with inheritance
1 parent c2bbf1a commit f530f91

File tree

12 files changed

+236
-179
lines changed

12 files changed

+236
-179
lines changed

‎src/main/java/com/github/sylvainlaurent/maven/swaggervalidator/semantic/VisitedItemsHolder.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.github.sylvainlaurent.maven.swaggervalidator.semantic;
22

33
import java.util.ArrayDeque;
4+
import java.util.Collection;
5+
import java.util.Collections;
46
import java.util.Deque;
57

68
import com.github.sylvainlaurent.maven.swaggervalidator.semantic.node.VisitableModel;
@@ -31,4 +33,8 @@ public void pop() {
3133
public String getCurrentPath() {
3234
return String.join(".", (Iterable<String>) () -> visitedItems.descendingIterator());
3335
}
36+
37+
public Collection<String> getVisitedItems() {
38+
return Collections.unmodifiableCollection(visitedItems);
39+
}
3440
}

‎src/main/java/com/github/sylvainlaurent/maven/swaggervalidator/semantic/node/model/ComposedModelWrapper.java‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package com.github.sylvainlaurent.maven.swaggervalidator.semantic.node.model;
22

3-
import java.util.ArrayList;
43
import java.util.List;
54
import java.util.Map;
65

@@ -64,16 +63,6 @@ public String getDiscriminator() {
6463
return ((ModelImpl) model.getChild()).getDiscriminator();
6564
}
6665

67-
public List<String> getRequired() {
68-
if (model.getChild() == null || !(model.getChild() instanceof ModelImpl)) {
69-
return new ArrayList<>();
70-
}
71-
// looks like it's always of this type
72-
List<String> required = ((ModelImpl) model.getChild()).getRequired();
73-
74-
return required == null ? new ArrayList<>() : required;
75-
}
76-
7766
// returns only properties from this model, not parents
7867
@Override
7968
public Map<String, VisitableProperty<? extends Property>> getProperties() {

‎src/main/java/com/github/sylvainlaurent/maven/swaggervalidator/semantic/node/model/ModelImplWrapper.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
11
package com.github.sylvainlaurent.maven.swaggervalidator.semantic.node.model;
22

3-
import static java.util.Collections.emptyList;
43
import static org.apache.commons.collections4.ListUtils.emptyIfNull;
54
import static org.apache.commons.lang3.reflect.FieldUtils.readField;
65

76
import java.math.BigDecimal;
7+
import java.util.ArrayList;
88
import java.util.List;
99
import java.util.stream.Collectors;
1010

1111
import com.github.sylvainlaurent.maven.swaggervalidator.semantic.ModelVisitor;
1212
import com.github.sylvainlaurent.maven.swaggervalidator.semantic.VisitablePropertyFactory;
1313
import com.github.sylvainlaurent.maven.swaggervalidator.semantic.node.VisitableProperty;
14-
1514
import io.swagger.models.ModelImpl;
1615
import io.swagger.models.properties.Property;
1716

@@ -26,11 +25,13 @@ public ModelImplWrapper(String name, ModelImpl model) {
2625

2726
@SuppressWarnings("unchecked")
2827
public List<String> getRequired() {
28+
List<String> requiredProperties;
2929
try {
30-
return emptyIfNull((List<String>) readField(model, "required", true));
30+
requiredProperties = new ArrayList<>(emptyIfNull((List<String>) readField(model, "required", true)));
3131
} catch (IllegalAccessException ex) {
32-
return emptyList();
32+
requiredProperties = new ArrayList<>();
3333
}
34+
return requiredProperties;
3435
}
3536

3637
public List<String> getReadOlyProperties() {

‎src/main/java/com/github/sylvainlaurent/maven/swaggervalidator/semantic/node/path/OperationWrapper.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ public class OperationWrapper implements PathObject {
2525
private String summary;
2626
private String description;
2727
private String operationId;
28-
private List<String> consumes = new ArrayList<>();
29-
private List<String> produces = new ArrayList<>();
28+
private List<String> consumes;
29+
private List<String> produces;
3030
private List<SchemeWrapper> schemes = new ArrayList<>();
3131
private List<Map<String, List<String>>> security = new ArrayList<>();
3232
private Boolean deprecated;
@@ -46,10 +46,10 @@ public OperationWrapper(String name, Operation operation, PathWrapper path) {
4646
this.tags.addAll(operation.getTags());
4747
}
4848
if (operation.getConsumes() != null) {
49-
this.consumes.addAll(operation.getConsumes());
49+
this.consumes = new ArrayList<>(operation.getConsumes());
5050
}
5151
if (operation.getProduces() != null) {
52-
this.produces.addAll(operation.getProduces());
52+
this.produces = new ArrayList<>(operation.getProduces());
5353
}
5454
if (operation.getSecurity() != null) {
5555
this.security.addAll(operation.getSecurity());

‎src/main/java/com/github/sylvainlaurent/maven/swaggervalidator/semantic/validator/definition/InheritanceChainPropertiesValidator.java‎

Lines changed: 134 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,89 +1,178 @@
11
package com.github.sylvainlaurent.maven.swaggervalidator.semantic.validator.definition;
22

3+
import static com.github.sylvainlaurent.maven.swaggervalidator.semantic.VisitableModelFactory.createVisitableModel;
4+
import static org.apache.commons.lang3.StringUtils.isEmpty;
5+
6+
import java.util.ArrayList;
7+
import java.util.Collection;
8+
import java.util.List;
9+
import java.util.Map;
10+
import java.util.Set;
11+
12+
import com.github.sylvainlaurent.maven.swaggervalidator.semantic.VisitablePropertyFactory;
313
import com.github.sylvainlaurent.maven.swaggervalidator.semantic.node.VisitableModel;
14+
import com.github.sylvainlaurent.maven.swaggervalidator.semantic.node.VisitableProperty;
15+
import com.github.sylvainlaurent.maven.swaggervalidator.semantic.node.model.ArrayModelWrapper;
416
import com.github.sylvainlaurent.maven.swaggervalidator.semantic.node.model.ComposedModelWrapper;
517
import com.github.sylvainlaurent.maven.swaggervalidator.semantic.node.model.ModelImplWrapper;
618
import com.github.sylvainlaurent.maven.swaggervalidator.semantic.node.model.RefModelWrapper;
19+
import com.github.sylvainlaurent.maven.swaggervalidator.semantic.node.property.ArrayPropertyWrapper;
20+
import com.github.sylvainlaurent.maven.swaggervalidator.semantic.node.property.ObjectPropertyWrapper;
721
import com.github.sylvainlaurent.maven.swaggervalidator.semantic.validator.error.DefinitionSemanticError;
22+
import com.github.sylvainlaurent.maven.swaggervalidator.util.Util;
823
import io.swagger.models.Model;
924
import io.swagger.models.RefModel;
1025
import io.swagger.models.properties.Property;
1126

12-
import java.util.ArrayList;
13-
import java.util.Collection;
14-
import java.util.Collections;
15-
import java.util.List;
16-
import java.util.Map;
17-
18-
import static com.github.sylvainlaurent.maven.swaggervalidator.semantic.VisitableModelFactory.createVisitableModel;
19-
import static java.util.Collections.disjoint;
20-
2127
public class InheritanceChainPropertiesValidator extends ModelValidatorTemplate {
2228

2329
@Override
24-
public void validate(ComposedModelWrapper composedModel) {
25-
Collection<String> childProperties = composedModel.getChild() == null || composedModel.getChild().getProperties() == null ?
26-
Collections.<String>emptyList() :
27-
composedModel.getChild().getProperties().keySet();
28-
List<String> parentProperties = new ParentPropertiesCollector(composedModel).getParentProperties();
29-
30-
boolean childHasPropertiesAlreadyPresentInParent = !disjoint(childProperties, parentProperties);
31-
if (childHasPropertiesAlreadyPresentInParent) {
30+
public void validate(ModelImplWrapper modelImplWrapper) {
31+
List<String> objectProperties = new ArrayList<>(modelImplWrapper.getProperties().keySet());
32+
List<String> requiredProperties = modelImplWrapper.getRequired();
33+
34+
validateDiscriminator(modelImplWrapper.getDiscriminator(), requiredProperties, objectProperties);
35+
validateProperties(objectProperties, requiredProperties);
36+
37+
for (VisitableProperty<? extends Property> property : modelImplWrapper.getProperties().values()) {
38+
property.accept(this);
39+
}
40+
}
41+
42+
private void validateDiscriminator(String discriminator, List<String> requiredProperties, List<String> properties) {
43+
if (isEmpty(discriminator)) {
44+
return;
45+
}
46+
47+
if (!properties.contains(discriminator)) {
48+
validationErrors.add(new DefinitionSemanticError(holder.getCurrentPath(),
49+
"discriminator \"" + discriminator + "\" is not a property defined at this schema"));
50+
}
51+
52+
if (!requiredProperties.contains(discriminator)) {
53+
validationErrors.add(new DefinitionSemanticError(holder.getCurrentPath(),
54+
"discriminator property \"" + discriminator + "\" is not marked as required"));
55+
}
56+
}
57+
58+
private void validateProperties(Collection<String> objectProperties, List<String> required) {
59+
Set<String> duplicates = Util.findDuplicates(objectProperties);
60+
if (!duplicates.isEmpty()) {
3261
validationErrors.add(new DefinitionSemanticError(
33-
holder.getCurrentPath(), "following properties are already defined in ancestors: "
34-
+ findCommonProperties(childProperties, parentProperties)));
62+
holder.getCurrentPath(), "following properties are already defined in ancestors: " + duplicates));
3563
}
64+
65+
duplicates = Util.findDuplicates(required);
66+
if (!duplicates.isEmpty()) {
67+
validationErrors.add(new DefinitionSemanticError(holder.getCurrentPath(),
68+
"required property is defined multiple times: " + duplicates));
69+
}
70+
71+
if (objectProperties.containsAll(required)) {
72+
return;
73+
}
74+
75+
List<String> requiredButNotDefinedProperties = new ArrayList<>(required);
76+
requiredButNotDefinedProperties.removeAll(objectProperties);
77+
validationErrors.add(new DefinitionSemanticError(holder.getCurrentPath(),
78+
"required properties are not defined as object properties: "
79+
+ requiredButNotDefinedProperties));
3680
}
3781

38-
private List<String> findCommonProperties(Collection<String> propertyNames, Collection<String> parentPropertyNames) {
39-
List<String> commonProperties = new ArrayList<>(propertyNames);
40-
commonProperties.retainAll(parentPropertyNames);
41-
return commonProperties;
82+
@Override
83+
public void validate(ArrayModelWrapper arrayModelWrapper) {
84+
if (arrayModelWrapper.getModel().getItems() == null) {
85+
validationErrors
86+
.add(new DefinitionSemanticError(holder.getCurrentPath(), "'items' must be defined for an array"));
87+
return;
88+
}
89+
arrayModelWrapper.getItems().accept(this);
4290
}
4391

44-
final class ParentPropertiesCollector extends ModelValidatorTemplate {
92+
@Override
93+
public void validate(ComposedModelWrapper composedModelWrapper) {
94+
InheritanceChainPropertiesValidator.AllPropertiesCollector allPropertiesCollector =
95+
new InheritanceChainPropertiesValidator.AllPropertiesCollector(
96+
composedModelWrapper);
97+
List<String> objectProperties = allPropertiesCollector.getProperties();
98+
List<String> requiredProperties = allPropertiesCollector.getRequiredProperties();
99+
validateProperties(objectProperties, requiredProperties);
100+
if (composedModelWrapper.getChild() != null) {
101+
for (VisitableProperty<? extends Property> property : createVisitableModel(null, composedModelWrapper.getChild()).getProperties()
102+
.values()) {
103+
property.accept(this);
104+
}
105+
}
106+
}
45107

46-
private List<String> parentProperties = new ArrayList<>();
108+
@Override
109+
public void validate(ObjectPropertyWrapper objectProperty) {
110+
validateProperties(objectProperty.getProperties().keySet(), objectProperty.getRequiredProperties());
47111

48-
private VisitableModel root;
112+
for (Map.Entry<String, Property> property : objectProperty.getProperties().entrySet()) {
113+
VisitablePropertyFactory.createVisitableProperty(property.getKey(), property.getValue()).accept(this);
114+
}
115+
}
116+
117+
@Override
118+
public void validate(ArrayPropertyWrapper arrayProperty) {
119+
if (arrayProperty.getProperty().getItems() == null) {
120+
validationErrors
121+
.add(new DefinitionSemanticError(holder.getCurrentPath(), "'items' must be defined for an array"));
122+
return;
123+
}
124+
125+
arrayProperty.getItems().accept(this);
126+
}
49127

50-
ParentPropertiesCollector(VisitableModel root) {
51-
this.root = root;
128+
final class AllPropertiesCollector extends ModelValidatorTemplate {
129+
private List<String> properties = new ArrayList<>();
130+
private List<String> requiredProperties = new ArrayList<>();
131+
132+
AllPropertiesCollector(VisitableModel root) {
133+
root.accept(this);
52134
}
53135

54136
@Override
55137
public void validate(ModelImplWrapper modelImplWrapper) {
56-
parentProperties.addAll(modelImplWrapper.getProperties().keySet());
138+
properties.addAll(modelImplWrapper.getProperties().keySet());
139+
requiredProperties.addAll(modelImplWrapper.getRequired());
57140
}
58141

59142
@Override
60143
public void validate(RefModelWrapper refModelWrapper) {
61-
String ref = refModelWrapper.getSimpleRef();
62-
Model model = InheritanceChainPropertiesValidator.this.context.getDefinitions().get(ref);
63-
if (model != null) {
64-
createVisitableModel(ref, model).accept(this);
144+
String refName = refModelWrapper.getSimpleRef();
145+
if (holder.getVisitedItems().contains(refName)) {
146+
InheritanceChainPropertiesValidator.this.validationErrors
147+
.add(new DefinitionSemanticError(InheritanceChainPropertiesValidator.this.holder.getCurrentPath(),
148+
"cyclic reference '" + refName + "'."));
149+
} else {
150+
holder.push(refName);
151+
String ref = refModelWrapper.getSimpleRef();
152+
Model model = InheritanceChainPropertiesValidator.this.context.getDefinitions().get(ref);
153+
if (model != null) {
154+
createVisitableModel(ref, model).accept(this);
155+
}
65156
}
157+
66158
}
67159

68160
@Override
69161
public void validate(ComposedModelWrapper composedModelWrapper) {
70-
if (composedModelWrapper != root) {
71-
Map<String, Property> childProperties = composedModelWrapper.getChild()== null || composedModelWrapper.getChild().getProperties() == null ?
72-
Collections.<String, Property>emptyMap() :
73-
composedModelWrapper.getChild().getProperties();
74-
parentProperties.addAll(childProperties.keySet());
162+
if (composedModelWrapper.getChild() != null) {
163+
createVisitableModel(null, composedModelWrapper.getChild()).accept(this);
75164
}
76-
77165
for (RefModel element : composedModelWrapper.getInterfaces()) {
78166
createVisitableModel(element.getSimpleRef(), element).accept(this);
79167
}
80168
}
81169

82-
List<String> getParentProperties() {
83-
if (parentProperties.isEmpty()) {
84-
root.accept(this);
85-
}
86-
return parentProperties;
170+
List<String> getProperties() {
171+
return properties;
172+
}
173+
174+
List<String> getRequiredProperties() {
175+
return requiredProperties;
87176
}
88177
}
89-
}
178+
}

0 commit comments

Comments
 (0)