-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
swagger: Fix type's to avoid deserialize issues. #4413
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you actually tested all your changes?
swagger.json
Outdated
}, | ||
"attributes": { | ||
"type": "object", | ||
"additionalProperties": true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be correct way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this change, The type for attributes generated is
@SerializedName("attributes")
private Object attributes = null;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure it works correctly for other languages? Because properties are usually not objects, but primitive values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see attributes to be dictionary of primitive types as such I made additional properties to be object. so that we get data type for attributes as Map<String,Object>.
Ex:
- GT06N Position attributes
attributes: {iccid=8991000901255216, distance=0.0, totalDistance=9968.19, motion=false}
- Teltonika Position attributes
attributes: {priority=0.0, sat=4.0, event=0.0, ignition=false, motion=true, rssi=3.0, io200=0.0, gpsStatus=1.0, pdop=1.4000000000000001, hdop=1.4000000000000001, power=0.0, io24=7.0, battery=3.6910000000000003, io68=0.0, operator=40445.0, io199=0.0, io16=1492.0, distance=35.03, totalDistance=1705.4}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's only valid in Java. It might not be valid for some other language.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could check for javascript and python, this is how the client code is generated
Javascript:
if (data.hasOwnProperty('attributes')) {
obj['attributes'] = ApiClient.convertToType(data['attributes'], {'String': Object});
}
/**
* @member {Object.<String, Object>} attributes
*/
exports.prototype['attributes'] = undefined;
Python:
swagger_types = {
'id': 'int',
'device_id': 'int',
'protocol': 'str',
'device_time': 'datetime',
'fix_time': 'datetime',
'server_time': 'datetime',
'outdated': 'bool',
'valid': 'bool',
'latitude': 'float',
'longitude': 'float',
'altitude': 'float',
'speed': 'float',
'course': 'float',
'address': 'str',
'accuracy': 'float',
'network': 'dict(str, object)',
'attributes': 'dict(str, object)'
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't seem right. Primitive types are not objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed & Updated the commit. But the primitive type is different for each attribute so wanted to make it an object so it can be cast to desired primitive type after fetching at the client side.
swagger.json
Outdated
"type": "string" | ||
"type": "object", | ||
"additionalProperties": { | ||
"type": "object" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't this is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this change, The type for attributes generated is
@SerializedName("attributes")
private Map<String, Object> attributes = null;
I feel this is a better way to read and manage stuff at the client side.
162591b
to
63e21c4
Compare
63e21c4
to
0bcdac4
Compare
Merged, thanks for the pull request. |
Refer to this post for discussion.