Support the required flag on resource properties (Discovery and Swagger)

Clément Denis · Clément Denis · commit d2c445511e0f · 2019-09-10T12:12:47.000+02:00
diff --git a/README.md b/README.md
@@ -28,14 +28,19 @@ To install test versions to Maven for easier dependency management, simply run:
 
 These are the most notable additions to
 [the original project by Google](https://github.com/cloudendpoints/endpoints-java), currently
-inactive: 
-- Allow adding [arbitrary data](https://github.com/AODocs/endpoints-java/pull/20) to generic errors
-- [Improve errors](https://github.com/AODocs/endpoints-java/pull/30) on malformed JSON
-- Generated Swagger spec is [compatible](https://github.com/AODocs/endpoints-java/pull/34) with 
-[Cloud Endpoints Portal ](https://cloud.google.com/endpoints/docs/frameworks/dev-portal-overview)
-([and](https://github.com/AODocs/endpoints-java/pull/38) 
-[other](https://github.com/AODocs/endpoints-java/pull/36) 
-[improvements](https://github.com/AODocs/endpoints-java/pull/37))
+inactive:
+- Runtime
+  - Allow [adding arbitrary data](https://github.com/AODocs/endpoints-java/pull/20) to generic errors
+  - [Improve returned errors](https://github.com/AODocs/endpoints-java/pull/30) on malformed JSON
+- Discovery and Swagger
+  - [Add description on resources and resource usage as request body](https://github.com/AODocs/endpoints-java/commit/bbb1eff2bb9e7d28fc2ec17599257d0ef610531d)
+  - [Support declaring resource properties as required](https://github.com/AODocs/endpoints-java/pull/41)
+- Swagger
+  - Generated spec is [fully compatible](https://github.com/AODocs/endpoints-java/pull/34) with 
+[Cloud Endpoints Portal](https://cloud.google.com/endpoints/docs/frameworks/dev-portal-overview) (and is 100% valid Swagger spec)
+  - Support [multi-API service](https://github.com/AODocs/endpoints-java/pull/40/commits/1f18d2f64f1538e63a7836a5cd52ff639fc624fd) in Endpoints Management
+  - [New options](https://github.com/AODocs/endpoints-java/pull/37) to combine common parameters in same path, extract parameter refs at spec level, add error model description, customize spec title and description
+  - [Add description support](https://github.com/AODocs/endpoints-java/pull/40/commits/bbb1eff2bb9e7d28fc2ec17599257d0ef610531d) for resource and resource usage
 
 Check 
 [closed PRs](https://github.com/AODocs/endpoints-java/pulls?q=is%3Apr+sort%3Aupdated-desc+is%3Aclosed)
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/IoUtil.java b/endpoints-framework/src/main/java/com/google/api/server/spi/IoUtil.java
@@ -30,6 +30,7 @@
 import java.io.PushbackInputStream;
 import java.io.RandomAccessFile;
 import java.net.URL;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.zip.GZIPInputStream;
@@ -55,7 +56,7 @@ private IoUtil() {}
   public static String readResourceFile(Class<?> c, String fileName) throws IOException {
     URL url = c.getResource(fileName);
     StringBuilder sb = new StringBuilder();
-    BufferedReader in = new BufferedReader(new InputStreamReader(url.openStream()));
+    BufferedReader in = new BufferedReader(new InputStreamReader(url.openStream(), StandardCharsets.UTF_8));
     for (String line = in.readLine(); line != null; line = in.readLine()) {
       sb.append(line);
     }
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/config/ApiResourceProperty.java b/endpoints-framework/src/main/java/com/google/api/server/spi/config/ApiResourceProperty.java
@@ -34,11 +34,20 @@
   String name() default "";
 
   /**
-   * The description that the property represented by the annotated getter, setter, or field should appear
-   * as in the API.
+   * The description that the property represented by the annotated getter, setter, or field should
+   * appear as in the API.
    */
   String description() default "";
 
+  /**
+   * Whether or not the property represented by the annotated getter, setter or field is "required":
+   * - For requests, indicates the property is required for the resource to be accepted
+   * - For responses, indicates the property will be returned by the server (before applying
+   * partial response filtering)
+   * In both cases, this is only a "hint": this is not enforced in any way.
+   */
+  AnnotationBoolean required() default AnnotationBoolean.UNSPECIFIED;
+
   /**
    * Whether or not the property represented by the annotated getter, setter or field should be
    * ignored for the API.
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/config/ResourcePropertySchema.java b/endpoints-framework/src/main/java/com/google/api/server/spi/config/ResourcePropertySchema.java
@@ -30,6 +30,7 @@
 public class ResourcePropertySchema {
   private final TypeToken<?> type;
   private String description;
+  private Boolean required;
 
   private ResourcePropertySchema(TypeToken<?> type) {
     this.type = type;
@@ -56,7 +57,16 @@ public String getDescription() {
   public void setDescription(String description) {
     this.description = description;
   }
-  
+
+  public Boolean getRequired() {
+    return required;
+  }
+
+  public ResourcePropertySchema setRequired(Boolean required) {
+    this.required = required;
+    return this;
+  }
+
   /**
    * Returns a default resource property schema for a given type.
    *
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/config/annotationreader/ApiAnnotationIntrospector.java b/endpoints-framework/src/main/java/com/google/api/server/spi/config/annotationreader/ApiAnnotationIntrospector.java
@@ -49,6 +49,7 @@
 import java.util.List;
 import java.util.Map;
 
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 /**
@@ -68,6 +69,23 @@ public boolean hasIgnoreMarker(AnnotatedMember member) {
     return apiProperty != null && apiProperty.ignored() == AnnotationBoolean.TRUE;
   }
 
+  @Override
+  public Boolean hasRequiredMarker(AnnotatedMember member) {
+    ApiResourceProperty apiProperty = member.getAnnotation(ApiResourceProperty.class);
+    Nonnull nonnull = member.getAnnotation(Nonnull.class);
+    Nullable nullable = member.getAnnotation(Nullable.class);
+    if (apiProperty != null && apiProperty.required() != AnnotationBoolean.UNSPECIFIED) {
+      return Boolean.parseBoolean(apiProperty.required().name());
+    }
+    if (nonnull != null) {
+      return true;
+    }
+    if (nullable != null) {
+      return false;
+    }
+    return null;
+  }
+
   @Override
   public PropertyName findNameForSerialization(Annotated a) {
     ApiResourceProperty apiName = a.getAnnotation(ApiResourceProperty.class);
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/config/jsonwriter/JacksonResourceSchemaProvider.java b/endpoints-framework/src/main/java/com/google/api/server/spi/config/jsonwriter/JacksonResourceSchemaProvider.java
@@ -72,6 +72,7 @@ public ResourceSchema getResourceSchema(TypeToken<?> type, ApiConfig config) {
         if (propertyType != null) {
           ResourcePropertySchema propertySchema = ResourcePropertySchema.of(propertyType);
           propertySchema.setDescription(definition.getMetadata().getDescription());
+          propertySchema.setRequired(definition.getMetadata().getRequired());
           schemaBuilder.addProperty(name, propertySchema);
         } else {
           logger.atWarning().log("No type found for property '%s' on class '%s'.", name, type);
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/Schema.java b/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/Schema.java
@@ -82,8 +82,12 @@ public static abstract class Field {
     /** The type classification of the field. */
     public abstract FieldType type();
 
+    /** The description of the field. */
     @Nullable public abstract String description();
 
+    /** The required status of the field. */
+    @Nullable public abstract Boolean required();
+
     /**
      * If {@link #type()} is {@link FieldType#OBJECT}, a reference to the schema type that the field
      * refers to.
@@ -106,6 +110,7 @@ public abstract static class Builder {
       public abstract Builder setName(String name);
       public abstract Builder setType(FieldType type);
       public abstract Builder setDescription(String description);
+      public abstract Builder setRequired(Boolean required);
       public abstract Builder setSchemaReference(SchemaReference ref);
       public abstract Builder setArrayItemSchema(Field schema);
       public abstract Field build();
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/SchemaRepository.java b/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/SchemaRepository.java
@@ -272,9 +272,11 @@ private Schema createBeanSchema(
       ResourcePropertySchema propertySchema = entry.getValue();
       TypeToken<?> propertyType = propertySchema.getType();
       if (propertyType != null) {
+        String description = propertySchema.getDescription();
         Field.Builder fieldBuilder = Field.builder()
             .setName(propertyName)
-            .setDescription(propertySchema.getDescription());
+            .setDescription(Strings.isNullOrEmpty(description) ? null : description)
+            .setRequired(propertySchema.getRequired());
         fillInFieldInformation(fieldBuilder, propertyType, typesForConfig, config);
         builder.addField(propertyName, fieldBuilder.build());
       }
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/discovery/DiscoveryGenerator.java b/endpoints-framework/src/main/java/com/google/api/server/spi/discovery/DiscoveryGenerator.java
@@ -279,6 +279,9 @@ private JsonSchema convertToDiscoverySchema(Field f) {
         .setType(f.type().getDiscoveryType())
         .setDescription(f.description())
         .setFormat(f.type().getDiscoveryFormat());
+    if (f.required() != null) {
+        fieldSchema.setRequired(f.required());
+    }
     if (f.type() == FieldType.ARRAY) {
       fieldSchema.setItems(convertToDiscoverySchema(f.arrayItemSchema()));
     }
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/discovery/UNSUPPORTED_FEATURES.md b/endpoints-framework/src/main/java/com/google/api/server/spi/discovery/UNSUPPORTED_FEATURES.md
@@ -0,0 +1,2 @@
+- Models:
+  - minimum,maximum,pattern,readonly,annotations
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/swagger/SwaggerGenerator.java b/endpoints-framework/src/main/java/com/google/api/server/spi/swagger/SwaggerGenerator.java
@@ -97,11 +97,8 @@
 import io.swagger.models.properties.PropertyBuilder;
 import io.swagger.models.properties.RefProperty;
 import io.swagger.models.properties.StringProperty;
-
 import io.swagger.models.refs.RefType;
 import java.lang.reflect.Type;
-import java.net.MalformedURLException;
-import java.net.URL;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -708,6 +705,9 @@ private Property convertToSwaggerProperty(Field f) {
     //the spec explicitly disallows description on $ref
     if (!(p instanceof RefProperty)) {
       p.description(f.description());
+      if (f.required() != null) {
+        p.setRequired(f.required());
+      }
     }
     return p;
   }
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/swagger/UNSUPPORTED_FEATURES.md b/endpoints-framework/src/main/java/com/google/api/server/spi/swagger/UNSUPPORTED_FEATURES.md
@@ -8,8 +8,6 @@
   - other repeated param features (uniqueItems, default values)
   - empty value parameters
   - headers in params
-- Models
-  - required properties (need new annotation property)
 - Responses
   - use introspection or new annotation to describe usage of any subclasses of ServiceException
   - headers in response
diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/config/model/SchemaRepositoryTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/config/model/SchemaRepositoryTest.java
@@ -9,8 +9,10 @@
 import com.google.api.server.spi.EndpointMethod;
 import com.google.api.server.spi.ServiceContext;
 import com.google.api.server.spi.TypeLoader;
+import com.google.api.server.spi.config.AnnotationBoolean;
 import com.google.api.server.spi.config.Api;
 import com.google.api.server.spi.config.ApiConfigLoader;
+import com.google.api.server.spi.config.ApiResourceProperty;
 import com.google.api.server.spi.config.Transformer;
 import com.google.api.server.spi.config.annotationreader.ApiConfigAnnotationReader;
 import com.google.api.server.spi.config.model.ApiParameterConfig.Classification;
@@ -23,6 +25,8 @@
 import com.google.common.reflect.TypeParameter;
 import com.google.common.reflect.TypeToken;
 
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -271,6 +275,62 @@ public void getOrAdd_recursiveSchema() throws Exception {
             .build());
   }
 
+  @Test
+  public void getOrAdd_requiredProperties() throws Exception {
+    TypeToken<RequiredProperties> type = TypeToken.of(RequiredProperties.class);
+    // This test checks the combinations of annotation that determine the "required" marker for
+    // resource properties.
+    repo.getOrAdd(type, config);
+    assertThat(repo.getOrAdd(type, config))
+        .isEqualTo(Schema.builder()
+            .setName("RequiredProperties")
+            .setType("object")
+            .addField("undefined", Field.builder()
+                .setName("undefined")
+                .setType(FieldType.STRING)
+                .build())
+            .addField("apiResourceProperty_undefined", Field.builder()
+                .setName("apiResourceProperty_undefined")
+                .setType(FieldType.STRING)
+                .build())
+            .addField("apiResourceProperty_required", Field.builder()
+                .setName("apiResourceProperty_required")
+                .setRequired(true)
+                .setType(FieldType.STRING)
+                .build())
+            .addField("apiResourceProperty_not_required", Field.builder()
+                .setName("apiResourceProperty_not_required")
+                .setRequired(false)
+                .setType(FieldType.STRING)
+                .build())
+            .addField("nullable", Field.builder()
+                .setName("nullable")
+                .setRequired(false)
+                .setType(FieldType.STRING)
+                .build())
+            .addField("nonnull", Field.builder()
+                .setName("nonnull")
+                .setRequired(true)
+                .setType(FieldType.STRING)
+                .build())
+            .addField("priority1", Field.builder()
+                .setName("priority1")
+                .setRequired(true)
+                .setType(FieldType.STRING)
+                .build())
+            .addField("priority2", Field.builder()
+                .setName("priority2")
+                .setRequired(true)
+                .setType(FieldType.STRING)
+                .build())
+            .addField("priority3", Field.builder()
+                .setName("priority3")
+                .setRequired(false)
+                .setType(FieldType.STRING)
+                .build())
+            .build());
+  }
+
   @Test
   public void get() {
     TypeToken<Parameterized<Integer>> type = new TypeToken<Parameterized<Integer>>() {};
@@ -401,6 +461,44 @@ public Parameterized<Short> transformFrom(Parameterized<String> in) {
     }
   }
 
+  private static class RequiredProperties {
+    public String getUndefined() {
+      return null;
+    }
+    @ApiResourceProperty
+    public String apiResourceProperty_undefined() {
+      return null;
+    }
+    @ApiResourceProperty(required = AnnotationBoolean.TRUE)
+    public String apiResourceProperty_required() {
+      return "";
+    }
+    @ApiResourceProperty(required = AnnotationBoolean.FALSE)
+    public String apiResourceProperty_not_required() {
+      return null;
+    }
+    @Nullable
+    public String getNullable() {
+      return null;
+    }
+    @Nonnull
+    public String getNonnull() {
+      return "";
+    }
+    @ApiResourceProperty(required = AnnotationBoolean.TRUE) @Nullable
+    public String getPriority1() {
+      return "";
+    }
+    @Nonnull @Nullable
+    public String getPriority2() {
+      return "";
+    }
+    @ApiResourceProperty(required = AnnotationBoolean.FALSE) @Nonnull
+    public String getPriority3() {
+      return null;
+    }
+  }
+
   private static class SelfReferencingObject {
     public SelfReferencingObject getFoo() {
       return null;
diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/discovery/DiscoveryGeneratorTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/discovery/DiscoveryGeneratorTest.java
@@ -44,6 +44,7 @@
 import com.google.api.server.spi.testing.NamespaceEndpoint;
 import com.google.api.server.spi.testing.NonDiscoverableEndpoint;
 import com.google.api.server.spi.testing.PrimitiveEndpoint;
+import com.google.api.server.spi.testing.RequiredPropertiesEndpoint;
 import com.google.api.services.discovery.model.DirectoryList;
 import com.google.api.services.discovery.model.RestDescription;
 import com.google.common.collect.ImmutableList;
@@ -219,6 +220,13 @@ public void testWriteDiscovery_FooEndpointWithDescription() throws Exception {
     compareDiscovery(expected, doc);
   }
 
+  @Test
+  public void testWriteDiscovery_RequiredPropertiesEndpoint() throws Exception {
+    RestDescription doc = getDiscovery(context, RequiredPropertiesEndpoint.class);
+    RestDescription expected = readExpectedAsDiscovery("required_parameters_endpoint.json");
+    compareDiscovery(expected, doc);
+  }
+  
   @Test
   public void testWriteDiscovery_multipleApisWithSharedSchema() throws Exception {
     // Read in an API that uses a resource with fields that have their own schema, then read in
diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/swagger/SwaggerGeneratorTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/swagger/SwaggerGeneratorTest.java
@@ -54,6 +54,7 @@
 import com.google.api.server.spi.testing.MultiResourceEndpoint.Resource2Endpoint;
 import com.google.api.server.spi.testing.MultiVersionEndpoint.Version1Endpoint;
 import com.google.api.server.spi.testing.MultiVersionEndpoint.Version2Endpoint;
+import com.google.api.server.spi.testing.RequiredPropertiesEndpoint;
 import com.google.api.server.spi.testing.SpecialCharsEndpoint;
 import com.google.common.collect.ImmutableList;
 
@@ -275,6 +276,14 @@ public void testWriteSwagger_FooEndpointWithDescription() throws Exception {
     checkSwagger(expected, swagger);
   }
 
+  @Test
+  public void testWriteSwagger_RequiredPropertiesEndpoint() throws Exception {
+    ApiConfig config = configLoader.loadConfiguration(ServiceContext.create(), RequiredPropertiesEndpoint.class);
+    Swagger swagger = generator.writeSwagger(ImmutableList.of(config), context);
+    Swagger expected = readExpectedAsSwagger("required_parameters_endpoint.swagger");
+    checkSwagger(expected, swagger);
+  }
+
   @Test
   public void testWriteSwagger_MultiResourceEndpoint() throws Exception {
     ServiceContext serviceContext = ServiceContext.create();
diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/swagger/SwaggerSubject.java b/endpoints-framework/src/test/java/com/google/api/server/spi/swagger/SwaggerSubject.java
diff --git a/endpoints-framework/src/test/resources/com/google/api/server/spi/discovery/required_parameters_endpoint.json b/endpoints-framework/src/test/resources/com/google/api/server/spi/discovery/required_parameters_endpoint.json
diff --git a/endpoints-framework/src/test/resources/com/google/api/server/spi/swagger/required_parameters_endpoint.swagger b/endpoints-framework/src/test/resources/com/google/api/server/spi/swagger/required_parameters_endpoint.swagger
diff --git a/test-utils/src/main/java/com/google/api/server/spi/testing/RequiredProperties.java b/test-utils/src/main/java/com/google/api/server/spi/testing/RequiredProperties.java
diff --git a/test-utils/src/main/java/com/google/api/server/spi/testing/RequiredPropertiesEndpoint.java b/test-utils/src/main/java/com/google/api/server/spi/testing/RequiredPropertiesEndpoint.java

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+- Models:`
	`2`	`+ - minimum,maximum,pattern,readonly,annotations`