[BUG][C++][Pistache-server] Array of string : The generator omits the "setter" and xxxIsSet()

[CyrilleBenard]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• What's the version of OpenAPI Generator used?
• Have you search for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Bounty to sponsor the fix (example)

Description

The generator does generates lots of usual code but the setter of the member variable (std::vectorstd::string type) is not here.

The getter is also "strange" or at least unsual because it returns the reference on the member without the qualification const, so that it may be modified.

Last, the usual function ClassName::memberNameIsSet() is not generated too.

On the opposite, lots of code is well generated and running, for example toJson and fromJson methods (except the the set of the bool variable m_xxxIsSet = true
unsetXXXX() is also well generated.

openapi-generator version

Current master 4.0.0-SNAPSHOT

OpenAPI declaration file content or url

openapi: 3.0.0
info:
  version: 1.0.0
  title: Check generation of array of object (Pistache)
  description: Internal ref filename is check_array_of_strings.yaml

servers:
  - url: http://localhost:8080

paths:
  /stair1:
    post:
      summary: blabla
      operationId: check_generation
      tags:
        - Stair1
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/UeContext'
        required: true
      responses:
        '200':
          description: Expected response to a valid request
          content:
            application/json:
              schema:
                type: string
        default:
          description: Unexpected error

components:
  schemas:
    UeContext:
      type: object
      properties:
        groups:
          type: array
          items:
            $ref: '#/components/schemas/GroupId'
          minItems: 0

    ObjectId:
      type: object
      properties:
        events:
          $ref: '#/components/schemas/GroupId'

    GroupId:
      type: string

Command line used for generation

Generate :

openapi-generator-cli.sh generate -i ./openapi.yaml -g cpp-pistache-server -c ./config.json -o .

Compile :

g++ -c  -I./api -I./model -I./impl -Wall -g -std=c++11 -o obj/model/UeContext.o model/UeContext.cpp

Steps to reproduce

Generate & compile

Related issues/PRs

N/A

Suggest a fix

EDIT: See below in the comment please

[auto-labeler]

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

[CyrilleBenard]

Ok I found the explanation inside the mustache file :

• No setter is due to the #isContainer section around the setter. I don't think this is correct so, I give below a patch that fix it
• No xxxxIsSet() method is due to the #required section. I think this is correct.

cd <openapi-generator ROOT folder>
patch -N -t -p0 < ./file.patch.txt

EDIT: Deleted patch file 'cause it was incomplete. See below the other comment

@etherealjoy let me know if I'm wrong (or not)

[etherealjoy]

No setter is due to the #isContainer section around the setter. I don't think this is correct so, I give below a patch that fix it

Array of strings is a container, so this is correct, there is a regression some where else

[CyrilleBenard]

Yes you are right but I misspoke :

I meant I wonder why the setter should not be generated when the type is a Container. No setter for an array of strings for example ? So, how should we set it without any setter :huh ?

Take care, my previous patch was not correct !
First, it affected also a getter and that was not my intention
Second, it was not sufficient because I omitted to modify the header file oops

More over, I had a look inside the cpp-qt5-qhttpengine-server corresponding model file and I found no constraint like that.

Please, have a look to the new patch and let me know if I missed something. It may occurs 😸

cd <openapi-generator ROOT folder>
patch -N -t -p1 < ./file.patch.txt

file.patch.txt

[etherealjoy]

For the container it not a problem because it is not a const for that purpose. To be able to get the map or array and then you can append or clear. This is how it is in pistache.
Converting it to the cpp-qt5-qhttpengine-server is also fine in my opinion... But it breaks code 🗡

[etherealjoy]

In the Qt5 server the return is a const. so you cannot append or clear anything.

QList<QString> OAIPet::getPhotoUrls() const {
    return photo_urls;
}

The only way to set it is to use the setter

void OAIPet::setPhotoUrls(const QList<QString> &photo_urls) 

In Pistache there is no const in the getter and also no setter because not needed

[CyrilleBenard]

I understand but that means the m_xxxIsSet member variable will never be true and thus, the to_json will never deserialize this container (it checks xxxIsSet() to do the job)

class  UeContext
{
public:
    UeContext();
    virtual ~UeContext();

    void validate();

    /////////////////////////////////////////////
    /// UeContext members

    /// <summary>
    /// 
    /// </summary>
    std::vector<std::string>& getGroups();
    bool groupsIsSet() const;
    void unsetGroups();

    friend void to_json(nlohmann::json& j, const UeContext& o);
    friend void from_json(const nlohmann::json& j, UeContext& o);
protected:
    std::vector<std::string> m_Groups;
    bool m_GroupsIsSet;
};

void to_json(nlohmann::json& j, const UeContext& o)
{
    j = nlohmann::json();
    if(o.groupsIsSet())
        j["groups"] = o.m_Groups;
}

Something is still blocking (in my brain) ^^

[etherealjoy]

We can change to the cpp-qt5-qhttpengine-server behaviour which better but lets ask also @stkrwork
This way we use const references everywhere also for maps and arrays

[CyrilleBenard]

I don't know if it would be the best solution but at least, something have to be modified cause, currently, the to_json method does not work with a container (see my comment above).

I understand why the current return is a reference (not const). It gives the opportunity to add, delete, modify some items inside the container without destroying all the current container to set another copy of another instance. That make sens but at least, again, we need to modify something to make the to_json working.

EDIT: If we want to let the code as it exist today, what about adding a method as below :

void Class::setXxxx(void) 
{
    m_XxxxIsSet = true ;
}

To mark the container Xxxx set and thus, make the to_json method work. This would be the opposite of the already coded method :

void Class::unsetXxxx(void)
{
    m_XxxxIsSet = false;
}

I think we also should do some modifications in the model getter (not container). I notice that all getters returned a copy of internal objects... ouchh, the performances are so low regarding a const ref return. I will open another ticket for that.

[etherealjoy]

I think we also should do some modifications in the model getter (not container). I notice that all getters returned a copy of internal objects... ouchh, the performances are so low regarding a const ref return. I will open another ticket for that.

I think there is a ticket for this, #1175

[CyrilleBenard]

You know all the opened bug issues ? :D
If I correctly understand the issue you linked, my intention is a little bit different. I especially had a look on your comment describing what you have done on the cpp-qt5-qhttpengine-server.
For example, without having checked how this class is designed :

QDateTime getShipDate() const;

My intention would be to defined it as below :

QDateTime & getShipDate() const;

This will avoid a temporary object construction.

When we are able to write some kind of code :

object1.get().doSomething().doAnotherThing().andSoOn() ;

The reference return is mandatory instead of a copy, in my opinion.
But that's not the current bug issue purpose, isn't it ? :=)

[etherealjoy]

Getting a ref is not always good, because sometimes the object being held gets descoped somewhere else especially for server side when handling multiple requests on different threads.

But that's not the current bug issue purpose, isn't it ? :=)

For this issue we could do the isSet and const ref param in the setter and avoid returning ref to object member