Skip to content

[csharp,java] Generated output includes Object.cs which produce compile error an 'Object' is an ambiguous reference #2064

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

Closed
ghost opened this issue Feb 8, 2016 · 29 comments

Comments

@ghost
Copy link

ghost commented Feb 8, 2016

Both v 2.1.5 and todays master (8 feb 2016) generates - for my project - a dummy Object file with no properties that gives compile errors unless it is deleted. Problem applies to both Java and CSharp version but below I will focus on the CSharp error. The does not happen with all projects (as an example, petstore works fine) but it happens with mine that was generated by Ahoy/swashbuckle. See attached file test-swagger_json.txt

For CSharp the generated dummy model is called Object and placed in an Object.cs file (see attachment below). It is not used and when removed the project will compile. So the problem seems to be that the file is generated at all.

Object.txt

@wing328
Copy link
Contributor

wing328 commented Feb 8, 2016

The endpoint ApiServicebureauByServicebureauIdEmployeersByEmployeerIdEmployeesByEmployeeIdPatch has body parameter referring '#/definitions/Object', which seems not defined (which means it's an object without property)

@ghost
Copy link
Author

ghost commented Feb 9, 2016

Yes, that could be why. Patch operations typically takes opaque input (which Ahoy/swashbuckle translates into Object for Java and C#). No Object file should be generate though as it refers to java.lang.Object (java) or System.Object (c#). The question is if the problem is in Ahoy/swashbuckle that fails the prefix the package/namespace to Object OR if the problem is in swagger-codegen that fails to filter out references to build-in system classes like Object ?

@wing328
Copy link
Contributor

wing328 commented Feb 9, 2016

I don't think #/definitions/Object translates to Object (in C#, Java or other langauges). It means a model named "Object" defined under definitions.

What does the body parameter look like? What about defining the body parameter as a proper model (with properties) as opposed to just a generic object?

@ghost
Copy link
Author

ghost commented Feb 9, 2016

In C# the type is a "dynamic" which is a type that can contain anything and be inspected at runtime for actual content. The corresponding method (ASP.NET 5 / MVC 6) is

public IActionResult MergeEmployee(uint servicebureauId, uint employeerId, uint employeeId, [FromBody]dynamic employeeChanges)

@wing328
Copy link
Contributor

wing328 commented Feb 9, 2016

Ideally you want to be able to supply any object but the question is how can these objects be serialized properly into JSON string (to be placed in the HTTP body)?

What does the HTTP body look like?

UPDATE: fix typo.

@ghost
Copy link
Author

ghost commented Feb 9, 2016

The HTTP body contains arbitrary json. The newtonsoft library which is used for serialization/deserialization can deserialize any json into a "dynamic". At runtime I can ask the dynamic type what properties it has etc. This is pretty nice for patch operations, since patch objects are not complete objects so I can't make a static type for the input.

@wing328
Copy link
Contributor

wing328 commented Feb 9, 2016

Thanks for sharing the requirement but that's not supported at the moment.

@ghost
Copy link
Author

ghost commented Feb 9, 2016

That is ok. I can work around it for now by deleting the file after generation or by adding "--import-mappings Object=System.Object"

@jimschubert
Copy link
Contributor

I've actually encountered this in C# by returning an anonymous object. For me, I think it's a result of Swashbuckle generating certain swagger definitions. If I remember correctly, I resolved the issue by creating a view model and annotating with the response:

[SwaggerResponse(HttpStatusCode.OK, Type=typeof(MyViewModel))]

I think a quick hacky fix would be to check returnType in the generator and if it's Object (and not System.Object), change it to modelPackage + ".Object". There could be a cli parameter to allow System.Object return types that doesn't do this fix.

@wing328
Copy link
Contributor

wing328 commented Mar 5, 2016

@bluemmc @jimschubert I've made some improvement via #2317. Would you please pull the latest master and give it another try?

@ghost
Copy link
Author

ghost commented Mar 7, 2016

The new version is a bit strange. Now a file called "ObjectObject.cs" is generated and used instead of Object.cs. I expected "Newtonsoft.Json.Linq.JObject" instead. Also, the generated project will not compile but gives an error:

C:\temp3>compile.bat
Installing 'RestSharp 105.1.0'.
Installing 'Newtonsoft.Json 8.0.2'.
Successfully installed 'RestSharp 105.1.0'.
Successfully installed 'Newtonsoft.Json 8.0.2'.
'cp' is not recognized as an internal or external command,
operable program or batch file.
'cp' is not recognized as an internal or external command,
operable program or batch file.
Microsoft (R) Visual C# Compiler version 4.6.1055.0
for C# 5
Copyright (C) Microsoft Corporation. All rights reserved.

This compiler is provided as part of the Microsoft (R) .NET Framework, but only
supports language versions up to C# 5, which is no longer the latest version. Fo
r compilers that support newer versions of the C# programming language, see http
://go.microsoft.com/fwlink/?LinkID=533240

error CS0006: Metadata file 'bin/Newtonsoft.Json.dll' could not be found
error CS0006: Metadata file 'bin/RestSharp.dll' could not be found

@wing328
Copy link
Contributor

wing328 commented Mar 7, 2016

Based on your feedback, I've submitted #2324. Please kindly perform a test when you've time.

@ghost
Copy link
Author

ghost commented Mar 7, 2016

@wing328 Your change is not yet available for me to test it - if I do a git pull I get no changes.
Note also, that JObject was not refered in the generated interface for api endpoints. I expected JObject instead of the dummy ObjectObject object.

@jimschubert
Copy link
Contributor

@bluemmc can you explain why you'd expect JObject? If you look at your Swagger spec, have you defined JObject as your model?

When I was receiving this error, it was because I wasn't documenting ResponseType as void and Swashbuckle would generate a response model of just Object. Swagger generator does the right thing to see that model and use it, but it conflicts with System.Object.

I don't think it's possible to just assume Object means JObject, because that would prevent people with a domain type of Object from using the generator.

@ghost
Copy link
Author

ghost commented Mar 7, 2016

@jimschubert I would expect JObject (or dynamic) in the last parameter of the endpoint in the generated client just like it is on the corresponding controller method serverside (implemented using ASP.NET 5 RC1 + Swatchbuckle/Ahoy):

        // Server side controller method
        [HttpPatch]
        public IActionResult MergeEmployee(uint servicebureauId, uint employeerId, uint employeeId, [FromBody]dynamic employeeChanges)

@ghost
Copy link
Author

ghost commented Mar 8, 2016

Looking at swatchbuckle's generated swagger.json file for my example above, it appears that Swatchbuckle/Ahoy inserts a "#/definitions/Object" reference if either "dynamic" or JObject is used in the controller. Hence, not sure if there is a bug in swagger-codegen (not converting definitions/Object into JObject) or if the bug is in Swatchbuckle/Ahoy (not referencing JObject somehow instead) ?

@wing328 wing328 modified the milestones: v2.2.0, v2.1.6 Mar 19, 2016
@wing328
Copy link
Contributor

wing328 commented Mar 19, 2016

I don't think a plug-in should automatically insert "#/definitions/Object" for type: object without any property or $ref when generating the spec.

@ghost
Copy link
Author

ghost commented Mar 21, 2016

So if Swatchbuckle/Ahoy, what does swagger-codegen expect that should cause it to generate a reference to JObject correctly ?

@wing328
Copy link
Contributor

wing328 commented Mar 21, 2016

If the response is set to "type: object", then the response type of the method in C# should be Object

For your case, I think the HTTP response data is in JSON format but technically XML is also a valid format to represent the response data and therefore we cannot simply map the response to JObject

@ghost
Copy link
Author

ghost commented Mar 21, 2016

So according to you, I can raise a bug with Swatchbuckle/Ahoy saying that it should insert "#/definitions/JObject" rather then "#/definitions/Object" for types "Newtonsoft.Json.Linq.JObject" and type "dynamic" ?

@wing328
Copy link
Contributor

wing328 commented Mar 21, 2016

Am I understanding correctly that you must map "Newtonsoft.Json.Linq.JObject" in C# with "object: type" and you won't accept any workaround which does not map JObject with "object: type" directly?

@ghost
Copy link
Author

ghost commented Mar 21, 2016

Not sure I understand your question.... What I really want is that if my MVC/C# controller uses Newtonsoft.Json.Linq.JObject or dynamic as a parameter for a method (and this method is exposed by Swatchbuckle/Ahoy), then when I generate an API client using swagger-codegen the generated C# client method should also use Newtonsoft.Json.Linq.JObject or dynamic as a parameter. If I generate a Java client something else that is similar to Newtonsoft.Json.Linq.JObject should be used (which is NOT java.lang.Object).

@robschne
Copy link

The problem is with the case of the keyword object in the mustache files that generate c#. It should be lower case not upper case object. I modified the mustache file to a lower case object and the c# code builds and runs correctly. That is a work around for the fact that an Object upper case class is generated on void return types, for me at least. Also string should not be upper case in c#. That is the difference between the keyword type and a complex type class. The use of ReSharper points out the use of an upper case is incorrect.

@wing328
Copy link
Contributor

wing328 commented Mar 21, 2016

@bluemmc to meet your requirement, I would imagine some works need to be done in the swagger core to handle specifically "type: object"

@wing328
Copy link
Contributor

wing328 commented Mar 21, 2016

@robschne if the return type is "void", it should not generate an "Object upper case class". Can you share the spec for that particular endpoint?

@jimschubert
Copy link
Contributor

@robschne Resharper offers the suggestion to use lower case object because it doesn't require using System; or linking mscorlib during build. It has nothing to do with correctness.

Your fix would work for the object issue, until you have some other primitive that requires using System; and your generated Object will again be ambiguous.

@bluemmc (cc @wing328) You can extend Swashbuckle with an OperationFilter to update the return type to your desired type. Or, you can document with the ResponseType attribute. Rather than dynamic, I think you can use ExpandoObject or DynamicObject (concrete implementations for the dynamic keyword), then codegen just needs to add a using System.Dynamic; to the API and models mustache files to accommodate those dynamic objects.

@ghost
Copy link
Author

ghost commented Mar 21, 2016

Thanks for all the comments. As I understand the comments, there is a limitation in swagger core concerning opaque objects like JObject/dynamic/ExpandoObject/DynamicObject that can represents any form of json/xml. If I use opaque in my controller, I can not generate a correct client API.

@jimschubert (cc @wing328) I use JObject/dynamic as a method parameter not a return type, so I can not use ResponseType.

@robschne
Copy link

On the return type being void I could have been just trying to figure it out but either way there is some flawed logic on the use of Object in the generated code. By invalid I mean the case you're using. I also @jimschubert I have never had a conflict or seen an example of a conflict using lower case object. I have been doing c# programming since c# 1 and have always used, and only ever seen examples with lower case object.

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

No branches or pull requests

3 participants