Missing schema properties in models from interfaces

[iKadmium]

When a controller specifies its return type as an interface, NSwag generates an empty model for it. The following is an interface, IFixtureData:

public interface IFixtureData
{
    ushort Address { get; set; }
    FixtureDefinitionSkeleton Type { get; set; }
    FixtureOptions Options { get; set; }
    string Group { get; set; }
}

In the swagger.json file, the following is generated:

"IFixtureDefinition": {
  "type": "object",
  "x-abstract": true,
  "additionalProperties": false
}

All of my concrete classes are generated properly, but the interfaces are all like this.

[RicoSuter]

Not sure if this feature is actually implemented by NJsonSchema... we use Newtonsoft.Json's ContractResolver to enumerate properties - I'd expect that interfaces are also supported by the resolver... but we need to verify this in http://njsonschema.org with a unit test

[mjeanrichard]

This seems to be a Problem with NJsonSchema. Getter and Setter methods are abstract in an Interface and these are not generated by default (see https://github.com/RSuter/NJsonSchema/blob/f7224a3ee33333b3cd439df536f80c5d551dcb21/src/NJsonSchema/Generation/JsonSchemaGenerator.cs#L598 ).

Everything works fine if I just set JsonSchemaGeneratorSettings.GenerateAbstractProperties to true, but this might not work in all situations.

[RicoSuter]

NJsonSchema uses the Newtonsoft.Json contract resolver to enumerate properties and do "reflection" over DTOs: This is done to ensure that the schema and the serialized/deserialized data match... The option GenerateAbstractProperties overrides this behavior and manually uses reflection to add abstract/interface properties - but because it is not used often (also not recommended) it might come with some problems etc. but NJsonSchema would be the place to test and fix this issue...

[mjeanrichard]

Should I create a corresponding issue in NJsonSchema? I could probably help with writing the UnitTest and potential fix...

[RicoSuter]

I’ll move this issue over to NJS, but it would be great if you create a (WIP) PR with some tests there...

[RicoSuter]

Moved to NJS - please create a PR with tests (& maybe a fix) so we can discuss further...

[mjeanrichard]

I added a PR with one test to show the Problem. Also included a potential fix, but i am unsure if this might have any side effects.

[mjeanrichard]

@RSuter : Could you have a look at this fix? Is this the way to go?

[mjeanrichard]

I still have a problem with this. The Properties are now generated, but in an abstract class an can therefore not be deserialized. Am I doing something wrong or is this behavior correct?

In my API I have a Interface as a return type. The gerenated TypeScript Class contains all the Properties, but fromJS Method contains the Line throw new Error("The abstract class '...' cannot be instantiated.");

Everthing else seems to be there (e.g. the correct init method).

Can I specify somehow, that I want non abstract classes to be genereated for interfaces?

[RicoSuter]

Theres an option to turn of x-abstract generation

[mjeanrichard]

Could you point me to that option? I can just see the assignment of schema.IsAbstract in the JsonSchemaGenerator on line 352. But there seems to be no condition there...

[RicoSuter]

Ah, maybe i was mistaken and there is no such option. Would this solve the problem? I think just allowing abstract class instances would then break on the server as they cannot be deserialized... have you also had a look at the inheritance handling in tandem with abstract classes/interfaces?

[RicoSuter]

Can you create a pr with your tests for discussions?

[mjeanrichard]

Hmm, yes, I had a quick look at inheritance handling. I currently only use interfaces in return types of my API and have therefore no issue on the server side :-). But I do not use any inheritance (at least I don't want it to be exposed in my schema).

How is NJsonSchema involved on the server side?

Could it be a solution to generate interfaces and (optionally) an implementation of it to be used? Because just an abstract class (or an interface for that matter) cannot be used at all on the client side.

[RicoSuter]

NJS is only used for serializing inheritance otherwise its mainly used for generating schemas...

Can you provide a comprehensive sample for what you want to achieve?

[mjeanrichard]

Lets assume I have a Controller like this:

[RoutePrefix("users")]
public class UserController : ApiController
{
  [HttpGet]
  [Route("")]
  public async Task<IUser> GetCurrentUser()
  {
    return await GetCurrentUserAsync();
  }
}

With the IUser Interface as follows:

public interface IUser
{
  string Lastname { get; set; }
  string Firstname { get; set; }
}

Of course there is an implementation of this interface on the server somewhere, but I do not want to expose it and in my case the API-Layer does not even know about it.

I now want to be able to generate TypeScript-Clients that can call this API and handle the Response. Even if I had a Controller like this

[RoutePrefix("users")]
public class UserController : ApiController
{
  [HttpPost]
  [Route("")]
  public async Task UpdateUser(IUser user)
  {
    ...
  }
}

I think clients should be gernerated with concrete DTOs (not abstract ones). How this is deserialized into something meaningful on the server is the servers problem.

[RicoSuter]

I think clients should be gernerated with concrete DTOs (not abstract ones). How this is deserialized into something meaningful on the server is the servers problem.

I agree.

Need to think about what this was actually trying to solve. But i think x-abstract should only be generated in an inheritance tree (ie when there’s a discriminator etc) - otherwise omitted

[mjeanrichard]

Is the inheritance tree opt-in? Is it only relevant if the JsonInheritanceConverter is used? What would be a good way to detect this?

[RicoSuter]

Yes opt-in with JsonInheritanceConverter, so i’d say we only generate x-abstract if its in an inheritance with JsonInheritanceConverter on the type or any base types

[RicoSuter]

But i need to check the tests and the code whether that is the correct behavior - not on pc right now

[mjeanrichard]

Sounds good to me, thanks. Tell me if and how I can be of help.

[RicoSuter]

Should be quite simple to implement/change but i need to ensure that nothing breaks and there are no regressions

[mjeanrichard]

Any news on this?

[RicoSuter]

Closed by 4643d38

I've added JsonSchemaGeneratorSettings to turn abstract off completely and an JsonSchemaAbstractAttribute to explicitely opt-in or opt-out...

is that ok?

[mjeanrichard]

Thanks for the Update (sorry for being late, i did somehow not get an update from github).

I am not sure how I should use this.
Can I currently set the new Property of JsonSchemaGeneratorSettings from NSwag, or should I create a PR there to include this?
Also I cannot add the new Attribute to Interfaces, because it has a [AttributeUsage(AttributeTargets.Class)] Attribute. Is that on purpose?

[RicoSuter]

Can I currently set the new Property of JsonSchemaGeneratorSettings from NSwag, or should I create a PR there to include this?

Yes, should be available

Also I cannot add the new Attribute to Interfaces, because it has a [AttributeUsage(AttributeTargets.Class)] Attribute. Is that on purpose?

That's bad, we need to add that

[mjeanrichard]

Could you point me to this option? I cannot see this in NSwagStudio (UI) nor see where I would change that in the nswag.json file.

[RicoSuter]

The setting has not yet been added to the command (CLI) and UI, but you can already set it in the ASP.NET Core middleware settings...

[RicoSuter]

see RicoSuter/NSwag@1a43cb4

[mjeanrichard]

Cool, thanks a lot for your work!

[mmalyska]

Also I cannot add the new Attribute to Interfaces, because it has a [AttributeUsage(AttributeTargets.Class)] Attribute. Is that on purpose?

That's bad, we need to add that

So, when it will be added?

[RicoSuter]

Just added it.