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

constructors of classes derived from VersionCreateRequest and VersionLoadRequest broken #150

Open
timo opened this issue Mar 4, 2024 · 4 comments

Comments

@timo
Copy link

timo commented Mar 4, 2024

The commit 80b249b introduced this change to the end of the constructor of SingleEntityVersionCreateRequest and ComplexVersionCreateRequest:

-        VersionCreateRequest.__init__(self, branch, type, version_name)
+        VersionCreateRequest.__init__(self, *args, **kwargs)

This is at the end of the constructors of these classes. The first thing the superclass constructor does here is to set _branch, _type, and _version_name to None, and since these are not in args or in kwargs, they don't get set to the values they are meant to be (this is what the subclass constructor did just a few lines earlier).

The same problem is also in the VersionLoadRequest superclass and its derived classes.

@timo
Copy link
Author

timo commented Mar 4, 2024

This is quite possibly a problem in the swagger definition that spring exports for thingsboard. I have not seen anything that explicitly speaks against it, but this doesn't seem correct:

      "ComplexVersionCreateRequest": {
        "title": "ComplexVersionCreateRequest",
        "properties": {
          "branch": {...},
          "entityTypes": {...},
          "syncStrategy": {...},
          "type": {...},
          "versionName": {...}
        }, 
        "allOf": [
          {
            "$ref": "#/components/schemas/VersionCreateRequest"
          },...],
... }, ...
      "VersionCreateRequest": {
        "title": "VersionCreateRequest",
        "type": "object",
        "properties": {
          "branch": {...},
          "type": {...},
          "versionName": {...}
        }},

Is it okay to have the branch, type, and versionName properties duplicated in the parent and child classes? At least the swagger api spec doesn't have it like that in their examples.

@timo
Copy link
Author

timo commented Mar 4, 2024

a random find, SmsTwoFaAccountConfig also has this problem with the use_by_default parameter, which is probably much harder to actually notice is not working

@timo
Copy link
Author

timo commented Mar 4, 2024

AlarmNotificationRuleTriggerConfig as well. It's probably a good idea to just go through every mention of "allOf" in the swagger definition json and check the constructors for this pattern

@timo
Copy link
Author

timo commented Mar 4, 2024

Perhaps you are using an older version of swagger-api/swagger-codegen#11968 which at least now passes all the arguments to the superclass constructor and that should fix this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant