Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Required parameters not handled as required #443

Closed
cchafer opened this issue Feb 20, 2015 · 8 comments
Closed

Required parameters not handled as required #443

cchafer opened this issue Feb 20, 2015 · 8 comments

Comments

@cchafer
Copy link
Contributor

cchafer commented Feb 20, 2015

Hi,
At this time, DefaultCodegen doesn't handle rightly required parameters. All of them as marked as NOT required when trying to figure out their type.

This can lead to several problems when, like me, you want to define type type of not required items as Option[...] in Scala.

Here's a simple fix for this :

diff --git a/modules/swagger-codegen/src/main/java/com/wordnik/swagger/codegen/DefaultCodegen.java b/modules/swagger-codegen/src/main/java/com/wordnik/swagger/codegen/DefaultCodegen.java
index 9d908aa..965106e 100644
--- a/modules/swagger-codegen/src/main/java/com/wordnik/swagger/codegen/DefaultCodegen.java
+++ b/modules/swagger-codegen/src/main/java/com/wordnik/swagger/codegen/DefaultCodegen.java
@@ -774,6 +774,8 @@ public class DefaultCodegen {
         LOGGER.warn("warning!  Property type \"" + qp.getType() + "\" not found for parameter \"" + param.getName() + "\", using String");
         property = new StringProperty().description("//TODO automatically added by swagger-codegen.  Type was " + qp.getType() + " but not supported");
       }
+
+      property.setRequired(param.getRequired());
       CodegenProperty model = fromProperty(qp.getName(), property);
       p.collectionFormat = collectionFormat;
       p.dataType = model.datatype;
@fehguy
Copy link
Contributor

fehguy commented Feb 20, 2015

Can you put in a test or two to demonstrate this in a PR to develop_2.0 branch?

cchafer added a commit to cchafer/swagger-codegen that referenced this issue Feb 23, 2015
@webron
Copy link
Contributor

webron commented May 3, 2015

@cchafer - a bit confused here. You prepared #446 but closed it, and can't find another submission. Am I missing something or did you perhaps forget? :)

@cchafer
Copy link
Contributor Author

cchafer commented May 4, 2015

Hi,

To be honnest, this little modification has already been merged a few weeks ago, in another PR, about CodegenParameter missing some data (can't remember exactly which one).
So this PR could be closed (I thought it already was, in fact).
Do you want me to close it ?

@webron
Copy link
Contributor

webron commented May 4, 2015

I'm just going over the issues and cleaning out to get a better picture of what's needed for the final release. I don't keep track over all changes in the project, but if you say this issue can be closed, that's great.

@ghost
Copy link

ghost commented Jul 1, 2015

facing this issue right now (using Java server-side code generation)
part of my contract (definitions section):

"sampleRequest": {
    "type": "object",
    "required": [
        "a",
        "b"
    ],
    "properties": {
        "a": {
            "type": "string",
            "description": "A field"
        },
        "b": {
            "type": "string",
            "description": "B field"
        },
        "c": {
            "type": "string",
            "description": "C field"
        }
    }
}

generated code:

  /**
   * A
   **/
  @ApiModelProperty(required = true, value = "A")
  @JsonProperty("a")
  public String getA() {
    return a;
  }
  public void setA(String a) {
    this.a = a;
  }

i can add
@JsonProperty(value = "a", required=true)
manually but I will lose it during next model generation (we don't store generated classes in code (and in repo as well), it generated automatically as step of project build)

@wing328
Copy link
Contributor

wing328 commented Jul 23, 2015

@cchafer @sunrisejjj I wonder if you can test with the latest develop_2.0 branch and let us know if the issue still exists. Thanks!

@ghost
Copy link

ghost commented Jul 30, 2015

@wing328 issue still exists. Maybe issue related to model.mustache file?
Line

@JsonProperty("{{name}}")

should be changed to something like this

@JsonProperty({{#required}}required = {{required}}, {{/required}}value = "{{name}}")

?

@fehguy
Copy link
Contributor

fehguy commented Oct 25, 2015

So to be clear, this issue is really about the generated code not enforcing the required parameter, not declaring it incorrectly in the swagger.json. It's of course possible to do, but I suggest that's taken into your own templates, as the server generation isn't necessarily doing all validations for you. Please reopen if you disagree or need more advice on this.

@fehguy fehguy closed this as completed Oct 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants