Skip to content
This repository was archived by the owner on Jun 30, 2023. It is now read-only.

Commit 1e573c8

Browse files
author
Clément Denis
committed
Support more types of Map to generate "additionalProperties" schema:
- Handle subclasses of Map with concrete key and value (exclude others) - Exclude raw Map type - Better support nested Maps
1 parent 3ae1b77 commit 1e573c8

File tree

10 files changed

+259
-22
lines changed

10 files changed

+259
-22
lines changed

‎endpoints-framework/src/main/java/com/google/api/server/spi/config/model/SchemaRepository.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import com.google.common.collect.Multimap;
1818
import com.google.common.reflect.TypeToken;
1919

20-
import java.lang.reflect.ParameterizedType;
2120
import java.util.EnumSet;
2221
import java.util.List;
2322
import java.util.Map;
@@ -162,10 +161,11 @@ private Schema getOrCreateTypeForConfig(
162161
return ANY_SCHEMA;
163162
} else if (Types.isMapType(type)) {
164163
schema = MAP_SCHEMA;
165-
boolean isGenericMapType = type.getType() instanceof ParameterizedType;
164+
final TypeToken<Map<?, ?>> mapSupertype = type.getSupertype(Map.class);
165+
final boolean hasConcreteKeyValue = Types.isConcreteType(mapSupertype.getType());
166166
boolean forceJsonMapSchema = MapSchemaFlag.FORCE_JSON_MAP_SCHEMA.isEnabled();
167-
if (isGenericMapType && !forceJsonMapSchema) {
168-
schema = createMapSchema(type, typesForConfig, config).or(schema);
167+
if (hasConcreteKeyValue && !forceJsonMapSchema) {
168+
schema = createMapSchema(mapSupertype, typesForConfig, config).or(schema);
169169
}
170170
typesForConfig.put(type, schema);
171171
schemaByApiKeys.put(key, schema);
@@ -213,31 +213,31 @@ private void addSchemaToApi(ApiKey key, Schema schema) {
213213
}
214214

215215
private Optional<Schema> createMapSchema(
216-
TypeToken type, Map<TypeToken<?>, Schema> typesForConfig, ApiConfig config) {
217-
FieldType keyFieldType = FieldType.fromType(Types.getTypeParameter(type, 0));
216+
TypeToken<Map<?, ?>> mapType, Map<TypeToken<?>, Schema> typesForConfig, ApiConfig config) {
217+
FieldType keyFieldType = FieldType.fromType(Types.getTypeParameter(mapType, 0));
218218
boolean supportedKeyType = SUPPORTED_MAP_KEY_TYPES.contains(keyFieldType);
219219
if (!supportedKeyType) {
220-
String message = "Map field type '" + type + "' has a key type not serializable to String";
220+
String message = "Map field type '" + mapType + "' has a key type not serializable to String";
221221
if (MapSchemaFlag.IGNORE_UNSUPPORTED_KEY_TYPES.isEnabled()) {
222222
System.err.println(message + ", its schema will be JsonMap");
223223
} else {
224224
throw new IllegalArgumentException(message);
225225
}
226226
}
227-
TypeToken<?> valueTypeToken = Types.getTypeParameter(type, 1);
227+
TypeToken<?> valueTypeToken = Types.getTypeParameter(mapType, 1);
228228
FieldType valueFieldType = FieldType.fromType(valueTypeToken);
229229
boolean supportArrayValues = MapSchemaFlag.SUPPORT_ARRAYS_VALUES.isEnabled();
230230
boolean supportedValueType = supportArrayValues || valueFieldType != FieldType.ARRAY;
231231
if (!supportedValueType) {
232-
System.err.println("Map field type '" + type + "' "
232+
System.err.println("Map field type '" + mapType + "' "
233233
+ "has an array-like value type, its schema will be JsonMap");
234234
}
235235
if (!supportedKeyType || !supportedValueType) {
236236
return Optional.absent();
237237
}
238238
TypeToken<?> valueSchemaType = ApiAnnotationIntrospector.getSchemaType(valueTypeToken, config);
239239
Schema.Builder builder = Schema.builder()
240-
.setName(Types.getSimpleName(type, config.getSerializationConfig()))
240+
.setName(Types.getSimpleName(mapType, config.getSerializationConfig()))
241241
.setType("object");
242242
Field.Builder fieldBuilder = Field.builder().setName(MAP_UNUSED_MSG);
243243
fillInFieldInformation(fieldBuilder, valueSchemaType, null, typesForConfig, config);

‎endpoints-framework/src/main/java/com/google/api/server/spi/config/model/Types.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,22 @@
2121
import com.google.api.server.spi.config.ResourceTransformer;
2222
import com.google.api.server.spi.config.Transformer;
2323
import com.google.api.server.spi.response.CollectionResponse;
24+
import com.google.common.base.Predicate;
25+
import com.google.common.collect.BoundType;
2426
import com.google.common.collect.Iterables;
2527
import com.google.common.reflect.TypeToken;
2628

29+
import java.lang.reflect.GenericArrayType;
2730
import java.lang.reflect.ParameterizedType;
2831
import java.lang.reflect.Type;
2932
import java.lang.reflect.TypeVariable;
3033
import java.lang.reflect.WildcardType;
34+
import java.util.Arrays;
3135
import java.util.Collection;
3236
import java.util.Map;
3337

38+
import javax.annotation.Nullable;
39+
3440
/**
3541
* Utilities for dealing with type information.
3642
*/
@@ -59,6 +65,30 @@ public static boolean isMapType(TypeToken<?> type) {
5965
return type.isSubtypeOf(Map.class) && !isJavaClientEntity(type);
6066
}
6167

68+
/**
69+
* Returns true if this type is not parameterized, or has only concrete type variables (checked
70+
* recursively on parameterized type variables).
71+
*/
72+
public static boolean isConcreteType(Type type) {
73+
if (type instanceof ParameterizedType) {
74+
Type[] typeArguments = ((ParameterizedType) type).getActualTypeArguments();
75+
return Iterables.all(Arrays.asList(typeArguments), new Predicate<Type>() {
76+
@Override
77+
public boolean apply(@Nullable Type input) {
78+
return isConcreteType(input);
79+
}
80+
});
81+
}
82+
if (type instanceof GenericArrayType) {
83+
return isConcreteType(((GenericArrayType) type).getGenericComponentType());
84+
}
85+
if (type instanceof Class) {
86+
return true;
87+
}
88+
//matches instanceof TypeVariable and WildcardType
89+
return false;
90+
}
91+
6292
/**
6393
* Returns whether or not this type is a Google Java client library entity.
6494
*/

‎endpoints-framework/src/test/java/com/google/api/server/spi/config/model/SchemaRepositoryTest.java‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.junit.Before;
2727
import org.junit.Test;
2828

29+
import java.util.HashMap;
2930
import java.util.Map;
3031

3132
/**
@@ -106,6 +107,22 @@ public void getOrAdd_mapType() throws Exception {
106107
.build());
107108
}
108109

110+
@Test
111+
public void getOrAdd_mapSubType() throws Exception {
112+
Schema expectedSchema = Schema.builder()
113+
.setName("Map_String_String")
114+
.setType("object")
115+
.setMapValueSchema(Field.builder()
116+
.setName(SchemaRepository.MAP_UNUSED_MSG)
117+
.setType(FieldType.STRING)
118+
.build())
119+
.build();
120+
assertThat(repo.getOrAdd(getMethodConfig("getMyMap").getReturnType(), config))
121+
.isEqualTo(expectedSchema);
122+
assertThat(repo.getOrAdd(getMethodConfig("getMySubMap").getReturnType(), config))
123+
.isEqualTo(expectedSchema);
124+
}
125+
109126
@Test
110127
public void getOrAdd_mapTypeUnsupportedKeys() throws Exception {
111128
System.setProperty(IGNORE_UNSUPPORTED_KEY_TYPES.systemPropertyName, "true");
@@ -116,6 +133,34 @@ public void getOrAdd_mapTypeUnsupportedKeys() throws Exception {
116133
}
117134
}
118135

136+
@Test
137+
public void getOrAdd_NestedMap() throws Exception {
138+
Schema expectedSchema = Schema.builder()
139+
.setName("Map_String_Map_String_String")
140+
.setType("object")
141+
.setMapValueSchema(Field.builder()
142+
.setName(SchemaRepository.MAP_UNUSED_MSG)
143+
.setType(FieldType.OBJECT)
144+
.setSchemaReference(SchemaReference.create(repo, config,
145+
new TypeToken<Map<String, String>>() {}))
146+
.build())
147+
.build();
148+
assertThat(repo.getOrAdd(getMethodConfig("getNestedMap").getReturnType(), config))
149+
.isEqualTo(expectedSchema);
150+
}
151+
152+
@Test
153+
public void getOrAdd_ParameterizedMap() throws Exception {
154+
checkJsonMap("getParameterizedMap");
155+
checkJsonMap("getParameterizedKeyMap");
156+
checkJsonMap("getParameterizedValueMap");
157+
}
158+
159+
@Test
160+
public void getOrAdd_RawMap() throws Exception {
161+
checkJsonMap("getRawMap");
162+
}
163+
119164
@Test
120165
public void getOrAdd_mapTypeArrayValues() throws Exception {
121166
System.setProperty(SUPPORT_ARRAYS_VALUES.systemPropertyName, "true");
@@ -270,11 +315,43 @@ public Map<String[], String> getArrayStringMap() {
270315
return null;
271316
}
272317

318+
public MyMap getMyMap() {
319+
return null;
320+
}
321+
322+
public Map<String, Map<String, String>> getNestedMap() {
323+
return null;
324+
}
325+
326+
public <K, V> Map<K, V> getParameterizedMap() {
327+
return null;
328+
}
329+
330+
public <K> Map<K, String> getParameterizedKeyMap() {
331+
return null;
332+
}
333+
334+
public <V> Map<String, V> getParameterizedValueMap() {
335+
return null;
336+
}
337+
338+
public Map getRawMap() {
339+
return null;
340+
}
341+
342+
public MySubMap getMySubMap() {
343+
return null;
344+
}
345+
273346
public Parameterized<Short> getTransformed() {
274347
return null;
275348
}
276349
}
277350

351+
private static class MyMap extends HashMap<String, String> { }
352+
353+
private static class MySubMap extends MyMap { }
354+
278355
private static class Parameterized<T> {
279356
public T getFoo() {
280357
return null;

‎endpoints-framework/src/test/resources/com/google/api/server/spi/discovery/map_endpoint.json‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,17 @@
212212
"https://www.googleapis.com/auth/userinfo.email"
213213
]
214214
},
215+
"getMapSubclass": {
216+
"httpMethod": "GET",
217+
"id": "myapi.mapEndpoint.getMapSubclass",
218+
"path": "mapsubclass",
219+
"response": {
220+
"$ref": "Map_Boolean_Integer"
221+
},
222+
"scopes": [
223+
"https://www.googleapis.com/auth/userinfo.email"
224+
]
225+
},
215226
"getStringArrayMap": {
216227
"httpMethod": "GET",
217228
"id": "myapi.mapEndpoint.getStringArrayMap",
@@ -345,6 +356,9 @@
345356
"mapService": {
346357
"$ref": "MapEndpoint"
347358
},
359+
"mapSubclass": {
360+
"$ref": "Map_Boolean_Integer"
361+
},
348362
"stringArrayMap": {
349363
"$ref": "JsonMap"
350364
},
@@ -360,6 +374,14 @@
360374
},
361375
"type": "object"
362376
},
377+
"Map_Boolean_Integer": {
378+
"additionalProperties": {
379+
"format": "int32",
380+
"type": "integer"
381+
},
382+
"id": "Map_Boolean_Integer",
383+
"type": "object"
384+
},
363385
"Map_Boolean_String": {
364386
"additionalProperties": {
365387
"type": "string"

‎endpoints-framework/src/test/resources/com/google/api/server/spi/discovery/map_endpoint_legacy.json‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,17 @@
212212
"https://www.googleapis.com/auth/userinfo.email"
213213
]
214214
},
215+
"getMapSubclass": {
216+
"httpMethod": "GET",
217+
"id": "myapi.mapEndpoint.getMapSubclass",
218+
"path": "mapsubclass",
219+
"response": {
220+
"$ref": "JsonMap"
221+
},
222+
"scopes": [
223+
"https://www.googleapis.com/auth/userinfo.email"
224+
]
225+
},
215226
"getStringArrayMap": {
216227
"httpMethod": "GET",
217228
"id": "myapi.mapEndpoint.getStringArrayMap",
@@ -317,6 +328,9 @@
317328
"mapService": {
318329
"$ref": "MapEndpoint"
319330
},
331+
"mapSubclass": {
332+
"$ref": "JsonMap"
333+
},
320334
"stringArrayMap": {
321335
"$ref": "JsonMap"
322336
},

‎endpoints-framework/src/test/resources/com/google/api/server/spi/discovery/map_endpoint_with_array.json‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,17 @@
212212
"https://www.googleapis.com/auth/userinfo.email"
213213
]
214214
},
215+
"getMapSubclass": {
216+
"httpMethod": "GET",
217+
"id": "myapi.mapEndpoint.getMapSubclass",
218+
"path": "mapsubclass",
219+
"response": {
220+
"$ref": "Map_Boolean_Integer"
221+
},
222+
"scopes": [
223+
"https://www.googleapis.com/auth/userinfo.email"
224+
]
225+
},
215226
"getStringArrayMap": {
216227
"httpMethod": "GET",
217228
"id": "myapi.mapEndpoint.getStringArrayMap",
@@ -341,6 +352,9 @@
341352
"mapService": {
342353
"$ref": "MapEndpoint"
343354
},
355+
"mapSubclass": {
356+
"$ref": "Map_Boolean_Integer"
357+
},
344358
"stringArrayMap": {
345359
"$ref": "Map_String_StringCollection"
346360
},
@@ -356,6 +370,14 @@
356370
},
357371
"type": "object"
358372
},
373+
"Map_Boolean_Integer": {
374+
"additionalProperties": {
375+
"format": "int32",
376+
"type": "integer"
377+
},
378+
"id": "Map_Boolean_Integer",
379+
"type": "object"
380+
},
359381
"Map_Boolean_String": {
360382
"additionalProperties": {
361383
"type": "string"

0 commit comments

Comments
 (0)