Skip to content

Commit 95a199a

Browse files
committed
Merge pull request #1130 from tomtit/issue1120
Fixed #1120: Models handling has been reworked.
2 parents fbc78e9 + 3b06d82 commit 95a199a

File tree

12 files changed

+537
-340
lines changed

12 files changed

+537
-340
lines changed

modules/swagger-core/src/main/java/io/swagger/jackson/ModelResolver.java

Lines changed: 19 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import io.swagger.annotations.ApiModelProperty;
1313
import io.swagger.converter.ModelConverter;
1414
import io.swagger.converter.ModelConverterContext;
15-
import io.swagger.util.Json;
1615

1716
import io.swagger.models.ComposedModel;
1817
import io.swagger.models.Model;
@@ -22,11 +21,11 @@
2221
import io.swagger.models.properties.AbstractNumericProperty;
2322
import io.swagger.models.properties.ArrayProperty;
2423
import io.swagger.models.properties.MapProperty;
25-
import io.swagger.models.properties.ObjectProperty;
2624
import io.swagger.models.properties.Property;
2725
import io.swagger.models.properties.RefProperty;
2826
import io.swagger.models.properties.StringProperty;
2927
import org.apache.commons.lang3.StringUtils;
28+
3029
import org.slf4j.Logger;
3130
import org.slf4j.LoggerFactory;
3231

@@ -92,85 +91,15 @@ public Property resolveProperty(JavaType propType,
9291
if (propType.isContainerType()) {
9392
JavaType keyType = propType.getKeyType();
9493
JavaType valueType = propType.getContentType();
95-
if(keyType != null && valueType != null) {
96-
MapProperty mapProperty = new MapProperty();
97-
Property innerType = getPrimitiveProperty(_typeName(valueType));
98-
99-
if(innerType == null) {
100-
String propertyTypeName = _typeName(valueType);
101-
Model innerModel = context.resolve(valueType);
102-
if(innerModel != null) {
103-
if(!"Object".equals(propertyTypeName)) {
104-
innerType = new RefProperty(propertyTypeName);
105-
mapProperty.additionalProperties(innerType);
106-
property = mapProperty;
107-
}
108-
else {
109-
innerType = new ObjectProperty();
110-
mapProperty.additionalProperties(innerType);
111-
property = mapProperty;
112-
}
113-
}
114-
}
115-
else {
116-
mapProperty.additionalProperties(innerType);
117-
property = mapProperty;
118-
}
119-
}
120-
else if(valueType != null) {
121-
ArrayProperty arrayProperty = new ArrayProperty();
122-
Property innerType = getPrimitiveProperty(_typeName(valueType));
123-
if(innerType == null) {
124-
LOGGER.debug("no primitive property type from " + valueType);
125-
String propertyTypeName = _typeName(valueType);
126-
LOGGER.debug("using name " + propertyTypeName);
127-
if (valueType.isEnumType()) {
128-
property = new StringProperty();
129-
_addEnumProps(valueType.getRawClass(), property);
130-
arrayProperty.setItems(property);
131-
property = arrayProperty;
132-
}
133-
else if(!"Object".equals(propertyTypeName)) {
134-
Model innerModel = context.resolve(valueType);
135-
LOGGER.debug("got inner model " + Json.pretty(innerModel));
136-
if(innerModel != null) {
137-
LOGGER.debug("found inner model " + innerModel);
138-
// model name may be overriding what was detected
139-
if( StringUtils.isNotEmpty(innerModel.getReference())){
140-
propertyTypeName = innerModel.getReference();
141-
}
142-
else if(innerModel instanceof ModelImpl) {
143-
ModelImpl impl = (ModelImpl) innerModel;
144-
if(impl.getName() != null)
145-
propertyTypeName = impl.getName();
146-
}
147-
Class<?> cls = propType.getRawClass();
148-
if(_isSetType(cls))
149-
arrayProperty.setUniqueItems(true);
150-
151-
innerType = new RefProperty(propertyTypeName);
152-
arrayProperty.setItems(innerType);
153-
property = arrayProperty;
154-
}
155-
}
156-
else {
157-
LOGGER.debug("falling back to object type");
158-
innerType = new ObjectProperty();
159-
arrayProperty.setItems(innerType);
160-
property = arrayProperty;
161-
}
162-
}
163-
else {
164-
if(keyType == null) {
165-
Class<?> cls = propType.getRawClass();
166-
167-
if(_isSetType(cls))
168-
arrayProperty.setUniqueItems(true);
169-
}
170-
171-
arrayProperty.setItems(innerType);
172-
property = arrayProperty;
94+
if (keyType != null && valueType != null) {
95+
property = new MapProperty().additionalProperties(context.resolveProperty(valueType, new Annotation[] {}));
96+
} else if (valueType != null) {
97+
ArrayProperty arrayProperty =
98+
new ArrayProperty().items(context.resolveProperty(valueType, new Annotation[] {}));
99+
if (_isSetType(propType.getRawClass())) {
100+
arrayProperty.setUniqueItems(true);
173101
}
102+
property = arrayProperty;
174103
}
175104
}
176105

@@ -184,8 +113,7 @@ else if (_isOptionalType(propType)) {
184113
}
185114
else {
186115
// complex type
187-
String propertyTypeName = _typeName(propType);
188-
Model innerModel = context.resolve(propType);
116+
Model innerModel = context.resolve(propType);
189117
if(innerModel instanceof ModelImpl) {
190118
ModelImpl mi = (ModelImpl) innerModel;
191119
property = new RefProperty( StringUtils.isNotEmpty( mi.getReference() ) ? mi.getReference() : mi.getName());
@@ -231,20 +159,22 @@ protected void _addEnumProps(Class<?> propClass, Property property) {
231159

232160

233161
public Model resolve(JavaType type, ModelConverterContext context, Iterator<ModelConverter> next) {
234-
final BeanDescription beanDesc = _mapper.getSerializationConfig().introspect(type);
235-
if (type.isEnumType()) {
236-
// TODO how to handle if model provided is simply an enum
162+
if (type.isEnumType() || _typeNameResolver.isStdType(type)) {
163+
// We don't build models for primitive types
164+
return null;
237165
}
238-
166+
if (type.isContainerType()) {
167+
// We treat collections as primitive types, just need to add models for values (if any)
168+
context.resolve(type.getContentType());
169+
return null;
170+
}
171+
final BeanDescription beanDesc = _mapper.getSerializationConfig().introspect(type);
239172
// Couple of possibilities for defining
240173
String name = _typeName(type, beanDesc);
241174

242175
if("Object".equals(name)) {
243176
return new ModelImpl();
244177
}
245-
if(type.isMapLikeType()) {
246-
return null;
247-
}
248178

249179
final ModelImpl model = new ModelImpl().type(ModelImpl.OBJECT).name(name)
250180
.description(_description(beanDesc.getClassInfo()));

modules/swagger-core/src/main/java/io/swagger/jackson/TypeNameResolver.java

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,18 +38,25 @@ public class TypeNameResolver {
3838
* specifically for Joda lib.
3939
*/
4040
protected final static Map<String, String> EXTERNAL_TYPES = externalTypes();
41-
42-
protected TypeNameResolver() { }
41+
42+
protected TypeNameResolver() {
43+
}
4344

4445
public String nameForType(JavaType type) {
4546
if (type.hasGenericTypes()) {
4647
return nameForGenericType(type);
4748
}
48-
final Class<?> raw = type.getRawClass();
49-
final String name = findStdName(raw);
50-
return (name == null) ? nameForClass(raw) : name;
49+
final String name = findStdName(type);
50+
return (name == null) ? nameForClass(type) : name;
51+
}
52+
53+
public boolean isStdType(JavaType type) {
54+
return findStdName(type) != null;
5155
}
5256

57+
protected String nameForClass(JavaType type) {
58+
return nameForClass(type.getRawClass());
59+
}
5360

5461
protected String nameForClass(Class<?> cls) {
5562
final ApiModel model = cls.getAnnotation(ApiModel.class);
@@ -58,18 +65,18 @@ protected String nameForClass(Class<?> cls) {
5865
}
5966

6067
protected String nameForGenericType(JavaType type) {
61-
final StringBuilder generic = new StringBuilder(nameForClass(type.getRawClass()));
68+
final StringBuilder generic = new StringBuilder(nameForClass(type));
6269
final int count = type.containedTypeCount();
6370
for (int i = 0; i < count; ++i) {
6471
final JavaType arg = type.containedType(i);
65-
final Class<?> argClass = arg.getRawClass();
66-
final String argName = findStdName(argClass) != null ? nameForClass(argClass) : nameForType(arg);
72+
final String argName = findStdName(arg) != null ? nameForClass(arg) : nameForType(arg);
6773
generic.append(WordUtils.capitalize(argName));
6874
}
6975
return generic.toString();
7076
}
7177

72-
protected String findStdName(Class<?> raw) {
78+
protected String findStdName(JavaType type) {
79+
final Class<?> raw = type.getRawClass();
7380
String name = JDK_TYPES.get(raw);
7481
if (name == null) {
7582
name = EXTERNAL_TYPES.get(raw.getName());
@@ -86,7 +93,7 @@ protected String findStdName(Class<?> raw) {
8693
}
8794
return name;
8895
}
89-
96+
9097
private static Map<Class<?>, String> jdkTypes() {
9198
Map<Class<?>, String> map = new HashMap<Class<?>, String>();
9299
_add(map, "boolean", Boolean.class, Boolean.TYPE);

modules/swagger-core/src/test/scala/ModelConverterTest.scala

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
1+
import scala.collection.JavaConverters.mapAsScalaMapConverter
2+
3+
import java.net.URI
4+
import java.net.URL
5+
import java.util.UUID
6+
17
import io.swagger.converter.ModelConverters
28
import io.swagger.models.ModelImpl
3-
import io.swagger.models.properties.{StringProperty, IntegerProperty, ArrayProperty, RefProperty}
9+
import io.swagger.models.properties.{StringProperty, IntegerProperty, ArrayProperty, MapProperty, RefProperty}
410
import io.swagger.util.Json
11+
512
import models._
613
import models.composition.Pet;
7-
import io.swagger.models._
8-
import io.swagger.models.properties._
9-
import io.swagger.converter._
1014

1115
import org.junit.runner.RunWith
1216
import org.scalatest.junit.JUnitRunner
@@ -182,7 +186,6 @@ class ModelConverterTest extends FlatSpec with Matchers {
182186

183187
val model = schemas.get("ClientOptInput")
184188
model.getProperties().size() should be (2)
185-
Json.prettyPrint(model)
186189
}
187190

188191
it should "set readOnly per #854" in {
@@ -193,11 +196,44 @@ class ModelConverterTest extends FlatSpec with Matchers {
193196
}
194197

195198
it should "process a model with org.apache.commons.lang3.tuple.Pair properties" in {
196-
ModelConverters.getInstance().addConverter(new ModelWithTuple2.TupleModelConverter(Json.mapper()))
197-
val schemas = ModelConverters.getInstance().readAll(classOf[ModelWithTuple2])
198-
val model = schemas.get("MyPair").asInstanceOf[ModelImpl]
199-
model.getType() should be ("object")
200-
model.getProperties() should be (null)
199+
val asMapCpnverter = new ModelWithTuple2.TupleAsMapModelConverter(Json.mapper());
200+
ModelConverters.getInstance().addConverter(asMapCpnverter)
201+
val asMap = ModelConverters.getInstance().readAll(classOf[ModelWithTuple2])
202+
ModelConverters.getInstance().removeConverter(asMapCpnverter)
203+
asMap.size() should be (4)
204+
for (pair <- List("MapOfString", "MapOfComplexLeft")) {
205+
val model = asMap.get(pair).asInstanceOf[ModelImpl]
206+
model.getType() should be ("object")
207+
model.getProperties() should be (null)
208+
model.getAdditionalProperties should not be (null)
209+
}
210+
211+
val asPropertyConverter = new ModelWithTuple2.TupleAsMapPropertyConverter(Json.mapper());
212+
ModelConverters.getInstance().addConverter(asPropertyConverter)
213+
val asProperty = ModelConverters.getInstance().readAll(classOf[ModelWithTuple2])
214+
ModelConverters.getInstance().removeConverter(asPropertyConverter)
215+
asProperty.size() should be (2)
216+
for ((name, property) <- asProperty.get("ModelWithTuple2").asInstanceOf[ModelImpl].getProperties.asScala) {
217+
name match {
218+
case "timesheetStates" =>
219+
property.getClass() should be (classOf[MapProperty])
220+
case "manyPairs" =>
221+
property.getClass() should be (classOf[ArrayProperty])
222+
property.asInstanceOf[ArrayProperty].getItems should not be (null)
223+
property.asInstanceOf[ArrayProperty].getItems.getClass should be (classOf[MapProperty])
224+
val items = property.asInstanceOf[ArrayProperty].getItems.asInstanceOf[MapProperty]
225+
items.getAdditionalProperties should not be (null)
226+
items.getAdditionalProperties.getClass should be (classOf[StringProperty])
227+
case "complexLeft" =>
228+
property.getClass() should be (classOf[ArrayProperty])
229+
property.asInstanceOf[ArrayProperty].getItems should not be (null)
230+
property.asInstanceOf[ArrayProperty].getItems.getClass should be (classOf[MapProperty])
231+
val items = property.asInstanceOf[ArrayProperty].getItems.asInstanceOf[MapProperty]
232+
items.getAdditionalProperties should not be (null)
233+
items.getAdditionalProperties.getClass should be (classOf[RefProperty])
234+
items.getAdditionalProperties.asInstanceOf[RefProperty].getSimpleRef should be ("ComplexLeft")
235+
}
236+
}
201237
}
202238

203239
it should "scan an empty model per 499" in {
@@ -222,7 +258,7 @@ class ModelConverterTest extends FlatSpec with Matchers {
222258
getClass.getMethods.toList.find { _.getName.equals("getGenericType") }.get.getGenericParameterTypes.toList(0)
223259
}
224260

225-
it should "check handling of Class<?> type" in {
261+
it should "check handling of the Class<?> type" in {
226262
val `type` = getGenericType()
227263
`type`.isInstanceOf[Class[_]] should be (false)
228264
val schemas = ModelConverters.getInstance().readAll(`type`)
@@ -264,4 +300,14 @@ class ModelConverterTest extends FlatSpec with Matchers {
264300
}"""
265301
)
266302
}
303+
304+
it should "check handling of string types" in {
305+
for (cls <- List(classOf[URI], classOf[URL], classOf[UUID])) {
306+
val schemas = ModelConverters.getInstance().readAll(cls)
307+
schemas.size should equal(0)
308+
val property = ModelConverters.getInstance().readAsProperty(cls)
309+
property should not be (null)
310+
property.getType should be ("string")
311+
}
312+
}
267313
}
Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,39 @@
11
package io.swagger.jackson;
22

3+
import java.lang.annotation.Annotation;
4+
import java.util.ArrayList;
5+
import java.util.Arrays;
6+
import java.util.Collection;
7+
8+
import org.hamcrest.CoreMatchers;
9+
import org.junit.Assert;
10+
11+
import com.google.common.base.Functions;
12+
import com.google.common.collect.Collections2;
13+
314
import io.swagger.converter.ModelConverterContextImpl;
415
import io.swagger.models.Model;
16+
import io.swagger.models.properties.Property;
17+
import io.swagger.models.properties.StringProperty;
518

619
public class EnumTest extends SwaggerTestBase {
7-
public enum Currency { USA, CANADA }
20+
public enum Currency {
21+
USA, CANADA
22+
}
823

924
public void testEnum() throws Exception {
10-
ModelResolver modelResolver = new ModelResolver(mapper());
25+
ModelResolver modelResolver = new ModelResolver(mapper());
1126
ModelConverterContextImpl context = new ModelConverterContextImpl(modelResolver);
12-
13-
Model model = context.resolve(Currency.class);
14-
assertNotNull(model);
1527

16-
// Set<String> names = model.getProperties().keySet();
17-
// if (names.contains("declaringClass")) {
18-
// TODO how best to handle this?
19-
// fail("Enum model should not contain property 'declaringClass', does; properties = " + names);
20-
// }
28+
Model model = context.resolve(Currency.class);
29+
assertNull(model);
30+
Property property = context.resolveProperty(Currency.class, new Annotation[] {});
31+
assertNotNull(property);
32+
Assert.assertThat(property, CoreMatchers.instanceOf(StringProperty.class));
33+
final StringProperty strProperty = (StringProperty) property;
34+
assertNotNull(strProperty.getEnum());
35+
final Collection<String> values =
36+
new ArrayList<String>(Collections2.transform(Arrays.asList(Currency.values()), Functions.toStringFunction()));
37+
assertEquals(values, strProperty.getEnum());
2138
}
22-
}
39+
}

0 commit comments

Comments
 (0)