You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@camel.apache.org by tomb50 <to...@gmail.com> on 2016/03/10 01:26:58 UTC
Bug in RestSwaggerReader.appendModels() - need confirmation
Hi,
I have encountered an issue relating to the Swagger component of Camel, I
believe it is related to the RestModelConvertor that is used when creating
the Swagger model from the Rest API model from Camel.
When a model that is directly referenced (e.g. an out-type) is added to the
Swagger.definitions map by that method, it is decorated with the extension
of "x-className". Any models that are subsequently referenced from the
original model are also added to the definitions map, undecorated.
Definitions needs to be decorated as such in order to be returned later as
ReferenceProperties in RestSwaggerReader.modelTypeAsRef(), otherwise they
are returned as StringProperties.
The issue seems to that there is nothing preventing a decorated element in
the Swagger.definitions map from being overwritten by an undecorated version
of the same class, if the REST API Verbs in Camel is ordered in a certain
way.
E.g.
API 1 returns Model A, which references Model B.
API 2 returns Model B.
Case 1 (no issue):
API 1 is loaded first.
Model A is added to Swagger.definition map decorated with "x-className",
Model B is added to the definition map undecorated.
API 2 is then loaded.
Model B is then decorated with "x-classname" and replaces the previous Model
B element in the definition map.
All good
Case 2 (the issue):
Consider that API 2 is loaded before API one, then after both API's are
loaded the definitions map contains a Decorated Model A but an undecorated
Model B.
Then when the RestSwaggerReader tries to set the schema on the response
element of API2 (towards the end of doParseVerbs()), it fails to find Model
B as a reference Property and adds it as a StringProperty, so the Model does
not show in Swagger UI and broke our validation tests.
We have many existing APIs and Models, all working perfectly, and I came
across this specific scenario today. I would appreciate If someone was able
to verify if this indeed is a defect or I am mistaken in my understanding.
If this is a bug, would a suitable fix to be to check the definition map for
an existing Model and NOT replace it, if it is decorated with the
"x-className" extension? Happy to get involved with a Pull request if
required.
Thanks
Tom
For ease: below is the code in question
---------------------------------------------
RestSwaggerReader.class
private void appendModels(Class clazz, Swagger swagger) {
RestModelConverters converters = new RestModelConverters();
final Map<String, Model> models = converters.readClass(clazz);
for (Map.Entry<String, Model> entry : models.entrySet()) {
swagger.model(entry.getKey(), entry.getValue());
}
}
-----------------------------------------------------------
RestModelConverters.class
public Map<String, Model> readClass(Class clazz) {
String name = clazz.getName();
Map<String, Model> resolved = super.read(clazz);
if (resolved != null) {
for (Model model : resolved.values()) {
// enrich with the class name of the model
model.getVendorExtensions().put("x-className", new
StringProperty(name));
}
// read any extra using read-all
Map<String, Model> extra = super.readAll(clazz);
if (extra != null) {
for (Map.Entry<String, Model> entry : extra.entrySet()) {
if (!resolved.containsKey(entry.getKey())) {
resolved.put(entry.getKey(), entry.getValue());
}
}
}
}
return resolved;
}
--
View this message in context: http://camel.465427.n5.nabble.com/Bug-in-RestSwaggerReader-appendModels-need-confirmation-tp5778821.html
Sent from the Camel - Users mailing list archive at Nabble.com.
Re: Bug in RestSwaggerReader.appendModels() - need confirmation
Posted by tomb50 <to...@gmail.com>.
bump
--
View this message in context: http://camel.465427.n5.nabble.com/Bug-in-RestSwaggerReader-appendModels-need-confirmation-tp5778821p5779210.html
Sent from the Camel Development mailing list archive at Nabble.com.
Re: Bug in RestSwaggerReader.appendModels() - need confirmation
Posted by Claus Ibsen <cl...@gmail.com>.
Hi
I logged a ticket
https://issues.apache.org/jira/browse/CAMEL-9732
On Fri, Mar 18, 2016 at 12:55 PM, Claus Ibsen <cl...@gmail.com> wrote:
> On Thu, Mar 10, 2016 at 1:26 AM, tomb50 <to...@gmail.com> wrote:
>> Hi,
>>
>> I have encountered an issue relating to the Swagger component of Camel, I
>> believe it is related to the RestModelConvertor that is used when creating
>> the Swagger model from the Rest API model from Camel.
>>
>> When a model that is directly referenced (e.g. an out-type) is added to the
>> Swagger.definitions map by that method, it is decorated with the extension
>> of "x-className". Any models that are subsequently referenced from the
>> original model are also added to the definitions map, undecorated.
>>
>> Definitions needs to be decorated as such in order to be returned later as
>> ReferenceProperties in RestSwaggerReader.modelTypeAsRef(), otherwise they
>> are returned as StringProperties.
>>
>> The issue seems to that there is nothing preventing a decorated element in
>> the Swagger.definitions map from being overwritten by an undecorated version
>> of the same class, if the REST API Verbs in Camel is ordered in a certain
>> way.
>>
>> E.g.
>>
>> API 1 returns Model A, which references Model B.
>> API 2 returns Model B.
>>
>> Case 1 (no issue):
>>
>> API 1 is loaded first.
>> Model A is added to Swagger.definition map decorated with "x-className",
>> Model B is added to the definition map undecorated.
>> API 2 is then loaded.
>> Model B is then decorated with "x-classname" and replaces the previous Model
>> B element in the definition map.
>>
>
> Yeah sounds better to keep existing. You are welcome to work on a PR
>
>> All good
>>
>> Case 2 (the issue):
>>
>> Consider that API 2 is loaded before API one, then after both API's are
>> loaded the definitions map contains a Decorated Model A but an undecorated
>> Model B.
>>
>> Then when the RestSwaggerReader tries to set the schema on the response
>> element of API2 (towards the end of doParseVerbs()), it fails to find Model
>> B as a reference Property and adds it as a StringProperty, so the Model does
>> not show in Swagger UI and broke our validation tests.
>>
>> We have many existing APIs and Models, all working perfectly, and I came
>> across this specific scenario today. I would appreciate If someone was able
>> to verify if this indeed is a defect or I am mistaken in my understanding.
>>
>> If this is a bug, would a suitable fix to be to check the definition map for
>> an existing Model and NOT replace it, if it is decorated with the
>> "x-className" extension? Happy to get involved with a Pull request if
>> required.
>>
>> Thanks
>> Tom
>>
>> For ease: below is the code in question
>>
>> ---------------------------------------------
>> RestSwaggerReader.class
>>
>> private void appendModels(Class clazz, Swagger swagger) {
>> RestModelConverters converters = new RestModelConverters();
>> final Map<String, Model> models = converters.readClass(clazz);
>> for (Map.Entry<String, Model> entry : models.entrySet()) {
>> swagger.model(entry.getKey(), entry.getValue());
>> }
>> }
>>
>>
>> -----------------------------------------------------------
>> RestModelConverters.class
>>
>> public Map<String, Model> readClass(Class clazz) {
>> String name = clazz.getName();
>> Map<String, Model> resolved = super.read(clazz);
>> if (resolved != null) {
>> for (Model model : resolved.values()) {
>> // enrich with the class name of the model
>> model.getVendorExtensions().put("x-className", new
>> StringProperty(name));
>> }
>>
>> // read any extra using read-all
>> Map<String, Model> extra = super.readAll(clazz);
>> if (extra != null) {
>> for (Map.Entry<String, Model> entry : extra.entrySet()) {
>> if (!resolved.containsKey(entry.getKey())) {
>> resolved.put(entry.getKey(), entry.getValue());
>> }
>> }
>> }
>> }
>> return resolved;
>> }
>>
>>
>>
>>
>>
>>
>>
>> --
>> View this message in context: http://camel.465427.n5.nabble.com/Bug-in-RestSwaggerReader-appendModels-need-confirmation-tp5778821.html
>> Sent from the Camel - Users mailing list archive at Nabble.com.
>
>
>
> --
> Claus Ibsen
> -----------------
> http://davsclaus.com @davsclaus
> Camel in Action 2: https://www.manning.com/ibsen2
--
Claus Ibsen
-----------------
http://davsclaus.com @davsclaus
Camel in Action 2: https://www.manning.com/ibsen2
Re: Bug in RestSwaggerReader.appendModels() - need confirmation
Posted by Claus Ibsen <cl...@gmail.com>.
On Thu, Mar 10, 2016 at 1:26 AM, tomb50 <to...@gmail.com> wrote:
> Hi,
>
> I have encountered an issue relating to the Swagger component of Camel, I
> believe it is related to the RestModelConvertor that is used when creating
> the Swagger model from the Rest API model from Camel.
>
> When a model that is directly referenced (e.g. an out-type) is added to the
> Swagger.definitions map by that method, it is decorated with the extension
> of "x-className". Any models that are subsequently referenced from the
> original model are also added to the definitions map, undecorated.
>
> Definitions needs to be decorated as such in order to be returned later as
> ReferenceProperties in RestSwaggerReader.modelTypeAsRef(), otherwise they
> are returned as StringProperties.
>
> The issue seems to that there is nothing preventing a decorated element in
> the Swagger.definitions map from being overwritten by an undecorated version
> of the same class, if the REST API Verbs in Camel is ordered in a certain
> way.
>
> E.g.
>
> API 1 returns Model A, which references Model B.
> API 2 returns Model B.
>
> Case 1 (no issue):
>
> API 1 is loaded first.
> Model A is added to Swagger.definition map decorated with "x-className",
> Model B is added to the definition map undecorated.
> API 2 is then loaded.
> Model B is then decorated with "x-classname" and replaces the previous Model
> B element in the definition map.
>
Yeah sounds better to keep existing. You are welcome to work on a PR
> All good
>
> Case 2 (the issue):
>
> Consider that API 2 is loaded before API one, then after both API's are
> loaded the definitions map contains a Decorated Model A but an undecorated
> Model B.
>
> Then when the RestSwaggerReader tries to set the schema on the response
> element of API2 (towards the end of doParseVerbs()), it fails to find Model
> B as a reference Property and adds it as a StringProperty, so the Model does
> not show in Swagger UI and broke our validation tests.
>
> We have many existing APIs and Models, all working perfectly, and I came
> across this specific scenario today. I would appreciate If someone was able
> to verify if this indeed is a defect or I am mistaken in my understanding.
>
> If this is a bug, would a suitable fix to be to check the definition map for
> an existing Model and NOT replace it, if it is decorated with the
> "x-className" extension? Happy to get involved with a Pull request if
> required.
>
> Thanks
> Tom
>
> For ease: below is the code in question
>
> ---------------------------------------------
> RestSwaggerReader.class
>
> private void appendModels(Class clazz, Swagger swagger) {
> RestModelConverters converters = new RestModelConverters();
> final Map<String, Model> models = converters.readClass(clazz);
> for (Map.Entry<String, Model> entry : models.entrySet()) {
> swagger.model(entry.getKey(), entry.getValue());
> }
> }
>
>
> -----------------------------------------------------------
> RestModelConverters.class
>
> public Map<String, Model> readClass(Class clazz) {
> String name = clazz.getName();
> Map<String, Model> resolved = super.read(clazz);
> if (resolved != null) {
> for (Model model : resolved.values()) {
> // enrich with the class name of the model
> model.getVendorExtensions().put("x-className", new
> StringProperty(name));
> }
>
> // read any extra using read-all
> Map<String, Model> extra = super.readAll(clazz);
> if (extra != null) {
> for (Map.Entry<String, Model> entry : extra.entrySet()) {
> if (!resolved.containsKey(entry.getKey())) {
> resolved.put(entry.getKey(), entry.getValue());
> }
> }
> }
> }
> return resolved;
> }
>
>
>
>
>
>
>
> --
> View this message in context: http://camel.465427.n5.nabble.com/Bug-in-RestSwaggerReader-appendModels-need-confirmation-tp5778821.html
> Sent from the Camel - Users mailing list archive at Nabble.com.
--
Claus Ibsen
-----------------
http://davsclaus.com @davsclaus
Camel in Action 2: https://www.manning.com/ibsen2