better validation for required fields

java/core/src/main/java/org/brapi/schematools/core/brapischema/BrAPISchemaReader.java

@@ -440,21 +440,32 @@ private Response<BrAPIType> createObjectType(Path path, JsonNode jsonNode, Strin
         List<String> required = findStringList(jsonNode, "required", false).getResultIfPresentOrElseResult(Collections.emptyList());
 
         List<BrAPIObjectProperty> properties = new ArrayList<>();
-        return Response.empty().
-            mapOnCondition(jsonNode.has("additionalProperties"), () -> findChildNode(jsonNode, "additionalProperties", true).
+        return Response.empty()
+            .mapOnCondition(jsonNode.has("additionalProperties"), () -> findChildNode(jsonNode, "additionalProperties", true).
                 mapResultToResponse(additionalPropertiesNode -> createProperty(path, additionalPropertiesNode, "additionalProperties",
-                    module, required.contains("additionalProperties")).onSuccessDoWithResult(properties::add))).
-            mapOnCondition(jsonNode.has("properties"), () -> findChildNode(jsonNode, "properties", true).
-                mapResult(JsonNode::fields).
-                mapResultToResponse(fields -> createProperties(path, fields, module, required)).
-                onSuccessDoWithResult(properties::addAll)).
-            onSuccessDo(() -> builder.properties(properties)).
-            mapOnCondition(jsonNode.has("brapi-metadata"), () -> findChildNode(jsonNode, "brapi-metadata", true).
-                mapResultToResponse(this::parseMetadata).onSuccessDoWithResult(builder::metadata)).
-            map(() -> success(builder.build()));
+                    module, required.contains("additionalProperties")).onSuccessDoWithResult(properties::add)))
+            .mapOnCondition(jsonNode.has("properties"), () -> findChildNode(jsonNode, "properties", true)
+                    .mapResult(JsonNode::fields)
+                .mapResultToResponse(fields -> createProperties(path, fields, module, required))
+                .onSuccessDoWithResult(properties::addAll))
+            .onSuccessDo(() -> builder.properties(properties))
+            .merge(validateRequiredProperties(required, properties, name))
+            .mapOnCondition(jsonNode.has("brapi-metadata"), () -> findChildNode(jsonNode, "brapi-metadata", true)
+                .mapResultToResponse(this::parseMetadata).onSuccessDoWithResult(builder::metadata))
+            .map(() -> success(builder.build()));
     }
 
+    private Response<List<BrAPIObjectProperty>> validateRequiredProperties(List<String> requiredPropertyNames, List<BrAPIObjectProperty> properties, String objectName) {
+        return requiredPropertyNames.stream().map(name -> validateRequiredProperty(name, properties, objectName)).collect(Response.toList());
+    }
 
+    private Response<BrAPIObjectProperty> validateRequiredProperty(String requiredPropertyName, List<BrAPIObjectProperty> properties, String objectName) {
+        return properties.stream().filter(property -> property.getName().equals(requiredPropertyName))
+            .findAny().map(Response::success)
+            .orElse(fail(Response.ErrorType.VALIDATION,
+                String.format("The required property '%s' is not found in the list of properties of '%s', expecting one of '%s", requiredPropertyName, objectName,
+                    properties.stream().map(BrAPIObjectProperty::getName).collect(Collectors.joining(", "))))) ;
+    }
 
     private Response<List<BrAPIObjectProperty>> createProperties(Path path, Iterator<Map.Entry<String, JsonNode>> fields, String module, List<String> required) {
         return Streams.stream(fields).map(field -> createProperty(path, field.getValue(), field.getKey(), module, required.contains(field.getKey()))).collect(Response.toList());

java/core/src/main/java/org/brapi/schematools/core/graphql/GraphQLGenerator.java

@@ -3,6 +3,7 @@
 import graphql.AssertException;
 import graphql.TypeResolutionEnvironment;
 import graphql.schema.*;
+import io.swagger.v3.oas.models.media.ObjectSchema;
 import lombok.AllArgsConstructor;
 import lombok.Getter;
 import org.brapi.schematools.core.brapischema.BrAPISchemaReader;
@@ -190,39 +191,6 @@ private boolean isInterface(BrAPIClass type) {
         private Response<GraphQLSchema> createSchema(List<GraphQLObjectType> primaryTypes) {
             GraphQLSchema.Builder builder = GraphQLSchema.newSchema();
 
-            if (options.isGeneratingQueryType()) {
-                GraphQLObjectType.Builder query = newObject().name(options.getQueryType().getName());
-
-                if (options.isGeneratingSingleQueries()) {
-                    primaryTypes.stream().filter(type -> options.getQueryType().getSingleQuery().isGeneratingFor(type.getName())).map(this::generateSingleGraphQLQuery).forEach(query::field);
-                }
-
-                if (options.isGeneratingListQueries()) {
-                    primaryTypes.stream().filter(type -> options.getQueryType().getListQuery().isGeneratingFor(type.getName())).map(this::generateListGraphQLQuery).forEach(query::field);
-                }
-
-                if (options.isGeneratingSearchQueries()) {
-                    primaryTypes.stream().filter(type -> options.getQueryType().getSearchQuery().isGeneratingFor(type.getName())).map(this::generateSearchGraphQLQuery).forEach(query::field);
-                }
-
-                builder.query(query);
-            }
-
-            if (options.isGeneratingMutationType()) {
-                GraphQLObjectType.Builder mutation = newObject().name(options.getMutationType().getName());
-
-                primaryTypes.stream().filter(type -> options.isGeneratingCreateMutationFor(type.getName()))
-                    .map(this::generateCreateGraphQLMutation).forEach(mutation::field);
-
-                primaryTypes.stream().filter(type -> options.isGeneratingUpdateMutationFor(type.getName()))
-                    .map(this::generateUpdateGraphQLMutation).forEach(mutation::field);
-
-                primaryTypes.stream().filter(type -> options.isGeneratingDeleteMutationFor(type.getName()))
-                    .map(this::generateDeleteGraphQLMutation).forEach(mutation::field);
-
-                builder.mutation(mutation);
-            }
-
             objectOutputTypes.values().forEach(builder::additionalType);
             interfaceTypes.values().forEach(builder::additionalType);
             enumTypes.values().forEach(builder::additionalType);
@@ -240,12 +208,54 @@ private Response<GraphQLSchema> createSchema(List<GraphQLObjectType> primaryType
             builder.codeRegistry(codeRegistry.build());
 
             try {
-                return success(builder.build());
+                return Response.empty()
+                    .mapOnCondition(options.isGeneratingQueryType(), () -> generateQueryType(primaryTypes).onSuccessDoWithResult(builder::query))
+                    .mapOnCondition(options.isGeneratingMutationType(), () -> generateMutationType(primaryTypes).onSuccessDoWithResult(builder::mutation))
+                    .withResult(builder.build()) ;
             } catch (AssertException e) {
                 return fail(Response.ErrorType.VALIDATION, e.getMessage());
             }
         }
 
+        private Response<GraphQLObjectType> generateQueryType(List<GraphQLObjectType> primaryTypes) {
+            GraphQLObjectType.Builder query = newObject().name(options.getQueryType().getName());
+
+            if (options.isGeneratingSingleQueries()) {
+                primaryTypes.stream().filter(type -> options.getQueryType().getSingleQuery().isGeneratingFor(type.getName())).map(this::generateSingleGraphQLQuery).forEach(query::field);
+            }
+
+            if (options.isGeneratingListQueries()) {
+                primaryTypes.stream().filter(type -> options.getQueryType().getListQuery().isGeneratingFor(type.getName())).map(this::generateListGraphQLQuery).forEach(query::field);
+            }
+
+            if (options.isGeneratingSearchQueries()) {
+                primaryTypes.stream().filter(type -> options.getQueryType().getSearchQuery().isGeneratingFor(type.getName())).map(this::generateSearchGraphQLQuery).forEach(query::field);
+            }
+
+            return success(query.build()) ;
+        }
+
+        private Response<GraphQLObjectType> generateMutationType(List<GraphQLObjectType> primaryTypes) {
+            GraphQLObjectType.Builder mutation = newObject().name(options.getMutationType().getName());
+
+            primaryTypes.stream()
+                .filter(type -> options.isGeneratingCreateMutationFor(type.getName()))
+                .map(this::generateCreateGraphQLMutation)
+                .forEach(mutation::field);
+
+            primaryTypes.stream()
+                .filter(type -> options.isGeneratingUpdateMutationFor(type.getName()))
+                .map(this::generateUpdateGraphQLMutation)
+                .forEach(mutation::field);
+
+            return primaryTypes.stream()
+                .filter(type -> options.isGeneratingDeleteMutationFor(type.getName()))
+                .map(this::generateDeleteGraphQLMutation)
+                .collect(Response.toList())
+                .withResult(mutation.build());
+
+        }
+
         private Response<GraphQLOutputType> createOutputType(BrAPIType type) {
 
             if (type instanceof BrAPIObjectType) {
@@ -726,12 +736,12 @@ private GraphQLOutputType creatSearchResponse(String queryName, GraphQLObjectTyp
         }
 
         private GraphQLFieldDefinition.Builder generateCreateGraphQLMutation(GraphQLObjectType type) {
-            return GraphQLFieldDefinition.newFieldDefinition()
+            return newFieldDefinition()
                 .name(options.getCreateMutationNameFor(type.getName()))
                 .description(createCreateMutationDescription(type))
                 .arguments(createCreateMutationArguments(type))
                 .type(options.getMutationType().getCreateMutation().isMultiple() ?
-                    GraphQLList.list(GraphQLTypeReference.typeRef(type.getName())) : GraphQLTypeReference.typeRef(type.getName())) ;
+                    GraphQLList.list(GraphQLTypeReference.typeRef(type.getName())) : GraphQLTypeReference.typeRef(type.getName()));
         }
 
         private String createCreateMutationDescription(GraphQLObjectType type) {
@@ -793,27 +803,36 @@ private List<GraphQLArgument> createUpdateMutationArguments(GraphQLObjectType ty
             return arguments;
         }
 
-        private GraphQLFieldDefinition.Builder generateDeleteGraphQLMutation(GraphQLObjectType type) {
-            return GraphQLFieldDefinition.newFieldDefinition()
+        private Response<GraphQLFieldDefinition.Builder> generateDeleteGraphQLMutation(GraphQLObjectType type) {
+            GraphQLFieldDefinition.Builder builder = newFieldDefinition()
                 .name(options.getDeleteMutationNameFor(type.getName()))
                 .description(createDeleteMutationDescription(type))
-                .arguments(createDeleteMutationArguments(type))
                 .type(options.getMutationType().getDeleteMutation().isMultiple() ?
                     GraphQLList.list(GraphQLTypeReference.typeRef(type.getName())) : GraphQLTypeReference.typeRef(type.getName())) ;
+
+            return createDeleteMutationArguments(type)
+                .onSuccessDoWithResult(builder::arguments)
+                .withResult(builder);
         }
 
         private String createDeleteMutationDescription(GraphQLObjectType type) {
             return options.getMutationType().getDeleteMutation().getDescriptionFor(type.getName());
         }
 
-        private List<GraphQLArgument> createDeleteMutationArguments(GraphQLObjectType type) {
+        private Response<List<GraphQLArgument>> createDeleteMutationArguments(GraphQLObjectType type) {
             List<GraphQLArgument> arguments = new ArrayList<>();
 
+            String idField = options.getIds().getIDFieldFor(type.getName()) ;
+
+            if (type.getFields().stream().noneMatch(field -> field.getName().equals(idField))) {
+                return fail(Response.ErrorType.VALIDATION, String.format("Can not find field '%s' in type '%s'", idField, type.getName())) ;
+            }
+
             GraphQLScalarType idType = options.isUsingIDType() ? GraphQLID : GraphQLString ;
 
             if (options.getMutationType().getDeleteMutation().isMultiple()) {
                 arguments.add(GraphQLArgument.newArgument().
-                    name(toPlural(options.getIds().getNameFor(type.getName()))).
+                    name(toPlural(idField)).
                     type(GraphQLList.list(idType)).
                     build());
             } else {
@@ -830,7 +849,7 @@ private List<GraphQLArgument> createDeleteMutationArguments(GraphQLObjectType ty
                     build());
             }
 
-            return arguments;
+            return success(arguments) ;
         }
     }
 

java/core/src/main/java/org/brapi/schematools/core/graphql/options/IdsOptions.java

@@ -6,6 +6,11 @@
 import org.brapi.schematools.core.model.BrAPIType;
 import org.brapi.schematools.core.utils.StringUtils;
 
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.brapi.schematools.core.utils.StringUtils.toParameterCase;
+
 /**
  * Provides options for the generation of Ids
  */
@@ -18,9 +23,12 @@ public class IdsOptions {
     String nameFormat;
     @JsonProperty("useIDType")
     boolean usingIDType;
+    @Setter(AccessLevel.PRIVATE)
+    private Map<String, String> fieldFor = new HashMap<>();
 
     public void validate() {
-        assert nameFormat != null : "'nameFormat' option on Mutation Ids Options is null";
+        assert nameFormat != null : String.format("'nameFormat' option on %s is null", this.getClass().getSimpleName());
+        assert fieldFor != null : String.format("'fieldFor' option on %s is null", this.getClass().getSimpleName());
     }
 
     /**
@@ -42,4 +50,29 @@ public final String getNameFor(@NonNull String name) {
     public final String getNameFor(@NonNull BrAPIType type) {
         return getNameFor(type.getName());
     }
+
+    /**
+     * Gets the id field name for a specific primary model. For example the id field
+     * name of Study, would be 'studyDbiId' by default. Use {@link #setIDParameterFor} to override this value.
+     * @param name the name of the primary model
+     * @return id parameter name for a specific primary model
+     */
+    @JsonIgnore
+    public String getIDFieldFor(String name) {
+        return fieldFor.getOrDefault(name, String.format(nameFormat, toParameterCase(name))) ;
+    }
+
+    /**
+     * Sets the id field name for a specific primary model. For example the id field
+     * name of Study, would be 'studyDbiId' by default.
+     * @param name the name of the primary model
+     * @param idField the id field name for a specific primary model.
+     * @return the options for chaining
+     */
+    @JsonIgnore
+    public IdsOptions setIDFieldFor(String name, String idField) {
+        fieldFor.put(name, idField) ;
+
+        return this ;
+    }
 }

java/core/src/main/resources/graphql-options.yaml

@@ -1,5 +1,11 @@
 pluralFor:
     AlleleMatrix: AlleleMatrix
+ids:
+    useIDType: true
+    nameFormat: "%sDbId"
+    fieldFor:
+        GermplasmAttribute: attributeDbId
+        GermplasmAttributeValue: attributeValueDbId
 input:
     name: input
     nameFormat: "%sInput"
@@ -69,6 +75,7 @@ mutationType:
             Event: false
             GenomeMap: false
             MarkerPosition: false
+            PedigreeNode: false
             Reference: false
             Variant: false
     updateMutation:
@@ -84,6 +91,7 @@ mutationType:
             Event: false
             GenomeMap: false
             MarkerPosition: false
+            PedigreeNode: false
             Reference: false
             Variant: false
     deleteMutation:
@@ -99,9 +107,7 @@ mutationType:
             Event: false
             GenomeMap: false
             MarkerPosition: false
+            PedigreeNode: false
             Reference: false
             Variant: false
-            VariantSet: false
-ids:
-    useIDType: true
-    nameFormat: "%sDbId"
+            VariantSet: false

java/core/src/test/resources/BrAPI-Schema/BrAPI-Common/Attribute.json

@@ -13,6 +13,10 @@
                             "type": "string",
                             "example": "Morphological"
                         },
+                        "attributeDbId": {
+                            "description": "The ID which uniquely identifies this attribute within the given database server",
+                            "type": "string"
+                        },
                         "attributeName": {
                             "description": "A human readable name for this attribute",
                             "type": "string",

java/core/src/test/resources/BrAPI-Schema/BrAPI-Genotyping/AlleleMatrix.json

@@ -169,8 +169,8 @@
                 }
             },
             "required": [
-                "callSetDbIds",
-                "variantSetDbIds"
+                "callSets",
+                "variantSets"
             ],
             "title": "AlleleMatrix",
             "type": "object",

java/core/src/test/resources/BrAPI-Schema/BrAPI-Genotyping/Call.json

@@ -87,9 +87,9 @@
                 }
             },
             "required": [
-                "callSetDbId",
-                "variantDbId",
-                "variantSetDbId"
+                "callSet",
+                "variant",
+                "variantSet"
             ],
             "title": "Call",
             "type": "object",

java/core/src/test/resources/BrAPI-Schema/BrAPI-Germplasm/GermplasmAttributeValue.json

@@ -50,8 +50,7 @@
                 }
             },
             "required": [
-                "attributeValueDbId",
-                "attributeName"
+                "attribute"
             ],
             "title": "GermplasmAttributeValue",
             "type": "object",