ServiceStack Reference generation in C# with nested classes

starafdar1 commented 3 months ago

Describe the issue

We recently upgraded from ServiceStack version 4 to version 8.2.2. Following the upgrade, we are encountering problems generating the ServiceStack references using the x tool. The problems are related to the usage of nested classes; we have a large number of nested classes in our DTOs for various child objects. (I realize that it's not a recommended practice as it creates problems with languages that do not support nested types, but we already have quite a few of these classes and it is difficult now to change them).

There are two scenarios I have identified so far:

Scenario 1: When a nested class is present and another class with the same name is present in the same namespace (either as a top-level class or nested under a different class), the reference generated includes only one of the classes and ignores the other classes. This causes a compile error on the client.

This seems to be due to this code in NativeTypesMetadata.cs:245 The code checks for duplicates against a hash containing the namspace and the class name. Therefore, classes are being excluded if the class name was already encountered in the namespace, even though it is at a different nesting level.

Scenario 2: When a class is derived from a generic class with a nested class as the generic argument, the code emitted does not decorate the nested class correctly. For example, for a class as follows:

public class Response1 : QueryResult<Response1.ResponseCore1>

The output being generated is

public class Response1 : QueryResult<ResponseCore1>

This also causes a compile error in the IDE.

This may be happening in the code in CSharpGeneratorExtensions.GetInherits() method. The code seems to be not taking into account the Declaring Type.

We have a large number of classes that follow this pattern. I am hoping that these issues can be fixed since moving all our nested classes to the top level will require a significant effort on our end.

Lastly, I wanted to confirm that while nested classes are not recommended, they are still supported in the latest versions of ServiceStack. I haven't seen any errors so far when calling the web services containing these nested classes, but I wanted to verify this.

Thanks.

Reproduction

Create a new self-hosted app project using x new selfhost NestedClasses
Add the NestedClasses.cs file from this gist to the ServiceModel project Gist
Add the methods below to the MyService.cs file.
Run x csharp https://localhost:5001/ or browse to https://localhost:5001/types/csharp to view the generated ServiceStack reference classes.

Methods to add to MyService.cs

    public object Any(Parent1 request)
    {
        return null;
    }

    public object Any(ServiceModel.Parent2 request)
    {
        return null;
    }

Expected behavior

The generated output should include the definition of the top-level Child1 class. It is not included, only the Parent1.Child1 class definition is included.
The Response1 class should be defined as public class Response1 : QueryResult<Response1.ResponseCore1>. Instead, it is defined as public class Response1 : QueryResult<ResponseCore1>.

System Info

We are on .Net Framework 4.8. I was also able to reproduce this in a .Net 8 sample app.
ServiceStack version is 8.2.2.

Additional context

No response

Validations

[X] Confirm Issue is reproducible from the information provided
[X] Have an active commercial ServiceStack License (GitHub Username is registered on Support page)

starafdar1 commented 2 months ago

Wondering if there is any feedback/update on this issue.

mythz commented 2 months ago

Inner classes are not compatible with Add ServiceStack Reference where all Request DTOs need to be unique and can't use any C# specific features, it only used to work for C# but no longer works after implementing support for AutoGen which needs to mix existing and code-generated types.

If you're using custom C# features like Inner classes you're going to be limited to sharing your ServiceModel.dll which should be in a impl-free that only references ServiceStack.Interfaces making it easy to share. Don't see any other solution other either manually fixing generated code or writing a tool that does it or copying the Source code of your ServiceModel folder.

Yeah Inner classes are definitely not recommended, DTOs are used to define your APIs public Service Contract, they should all be unique and able to invoke with just its Name and Request Body.

starafdar1 commented 2 months ago

Thank you for your reply.

Can the use of nested classes cause problems with web services as well? Or will the issue only arise when generating the reference classes?

I fully understand that choosing to use nested classes was not the right decision. However, removing these nested classes now would be extremely difficult. Besides the effort that will be involved, a major issue is that we have external clients who use our API. If they have already generated references to our API, their code will break if we change the class structure now. I don't think we will be able to convince our clients to accept such a change.

Is there any way to add back support for the nested classes in the reference class generation? It seems the issues are occurring in two areas of the code that I linked to in the original post. While I am not familiar with the ServiceStack code or how it interacts with AutoGen, it appears at first glance that the required changes would be limited in scope. The first issue involves modifying a duplicate check to consider the nesting level, and the second involves considering the parent type when emitting the code for declaring the base class.

Thanks.

mythz commented 2 months ago

They wont have an issue with your Service implementation or typed .NET to .NET integrations which use either your ServiceModel .dll or ServiceModel source code, but they wont be able to use Add ServiceStack Reference which is a cross-platform tool to generate typed integrations for 11 different languages.

We don't want to support inner classes in Add ServiceStack Reference since they're not compatible in non .NET languages, violate the Unique Request DTO Type Name constraint and should be avoided. This behavior has existed for years so don't want to add back the constraint of needing to support inner classes for a specific language now.

Why can't you give Customers that need your updated DTOs your ServiceModel .dll or Source code? The purpose of Add ServiceStack Reference is to recreate DTOs in different languages, which can't be used when using inner classes, so why not just give them the original .cs or .dll?

ServiceStack / Issues