-
Notifications
You must be signed in to change notification settings - Fork 36
Force Points Encoded = false #1
Comments
Related to this here? http://swagger.io/unlocking-the-spec-the-default-keyword/ or this swagger-api/swagger-core#1844 Will this work? - name: calc_points
type: boolean
default: false |
Ah, ok. It won't work: Also, I think you're missing the point of the default value. The default value is one that's assumed by the receiving end (either the server or the client) if the sender does not provide a value for it - it's not what automatically should be filled. |
Exactly :) That was my first try, as well. I also experimented with required=true and default=false, which does not work either. So right now I have the feeling, that swagger does not allow adding fixed values to the query, but maybe we find something. |
Maybe this answer somehow via |
And then there is |
Thanks, I will check if either of your proposed approaches work. |
The enum seems to only work for the swagger UI. It did not changed the generated code, I was able to still put true as value. The client send true to the server.
The same applies to the defaultValue. |
I added a question to Stackoverflow. Also I just had another idea. We could allow points_encoded=true and return the encoded string. That way a user has to decode the point on his/her own. |
Well, both should be possible but I would like to avoid the necessity to decode it for a fresh user. Maybe we should just document that per default you get the encoded polyline and how to decode it in JS/Java, and if the user does not want this he can switch to the array variant (this "either/or" should be possible in swagger) |
I've researched for a couple of while now and it seems like handling the points_encoded will be quite difficult. The reason is that swagger expects for a certain format for a certain key. In this example for points it would expect an array of coordinates:
Swagger supports the concept of polymorphism, but polymorphism requires a field with a string that contains the name of the of the type. See their examples and search for |
What do you mean? Isn't there an 'optional'-like possibility? |
Yes optional is allowed per default. What's not allowed easiy is to state something like: |
Hmmh, strange. Thought have seen this somewhere. |
I've no swagger spec knowledge :) ...but that looks exactly what we need - why would the following not work? ..
allOf:
- $ref: '#/definitions/PointsGeoJson'
- type: string
...
PointsGeoJson:
properties:
coordinates:
type: array
... |
I think the idea of allOf is something different. I think it's about defining a common set of properties. For example: every response contains a message of type string. This would allow you to add the allOf to every Response and therefore need only to define a "CommonResponseData" object. I tested it like this:
The editor validates this as valid swagger, but when I generate the client the points entry is completely missing. So at least the swagger-codegen does not understand it. I think what we look for is oneOf, which will be supported by OpenAPI 3.0
See for example this issue. There are tons of requests for that. I think we might be able to use Polymorphism. There is a swagger spec for GeoJson. I haven't tested it, but this should be what we need. But Polymorphism needs a type string, like: point, polygon etc. Swagger maps the object to the object that equals the type string. Not really elegant and we don't have something like this right now. We only have points_encoded: true|false. Not sure if we should name the objects true and false, and if this would even work. So eventually it would work if would return a point_type: PointsEncoded|PointsNotEncoded or something like this. Should I test this? |
Thanks for digging into this!
Yes, please test if an additional helper string or boolean would work. Or finding a solution (or an issue) for the default parameter could also help. |
Ok I tested Polymorphism. After I worte the swagger I found out that Polymorphism is not supported for Java clients, yet (not released see my comment below). I think Polymorphism is supported for other clients. The problem is, we could not use the Routing API from certain languages. Not supported languages:
It seems to be supported for PHP, Swift and ObjC. I opened another issue to see if we can get an overview of the supported languages to make a reasonable decision: swagger-api/swagger-codegen#4622. So in short: If we use this polymorphism approach not all clients will work. So the question is if we should follow this road or just support points_encoded=false? However, I still think there is no solution to force the parameter to false, which might lead to misuse of the client... Some related issues: Anyway the swagger is like that:
|
Nice - thanks!
Nice finding. Then let us for now go without polymorphism to bundle all APIs and support as many languages as possible. And only document the ugliness for using the Routing API... |
Ok, I would wait one or two days to see what languages are not supported with polymorphism (assuming that we get a fast answer to the issue :)). If it would be only Python and some almost not used languages I think it would be better to go with polymorphism. But if we cannot support 2-3 major languages we shouldn't use polymorphism. |
Ok, it seems that there are a couple of languages that miss the polymorphism support. Thus, I will model the swagger to only support unencoded points. |
How again was GeoJSON handled? Could we introduce a new |
GeoJson used the Polymorphism concept. The point_type would be the discriminator. See my code posted here. That might work, but only for some languages. Others don't support it. |
That seems exactly right, and we should put it in there. See here, scroll down to Constant Parameters. |
Yes I think so too, the issue was that it didn't work when I tried it. it would be nice if it would be working now :). |
Currently, points_encoded has to be manually set to false, which is not optimal.
The text was updated successfully, but these errors were encountered: