Skip to content
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

'Required' is an ambiguous reference between 'System.ComponentModel.DataAnnotations.RequiredAttribute' and 'Json.Schema.Generation.RequiredAttribute'? #702

Closed
2 tasks done
ptr727 opened this issue Apr 3, 2024 · 22 comments
Labels
pkg:schema-generation question Further information is requested

Comments

@ptr727
Copy link

ptr727 commented Apr 3, 2024

Documentation

  • I have consulted the documentation, and my question isn't answered there.

Nuget Package

JsonSchema.Net.Generation

Package Version

4.3.0

How can I help? Please provide as much context as possible.

Hi, I am migrating from newtonsoft to text.json, with some issues I am still trying to resolve along the way.
I am also looking for a more "native" text.json schema from code builder, and found jsonschema.net.generation as a replacement for newtonsoft.json.scheme.

(I am really looking for a configuration to build schemas for all nor [Obsolete] items (properties, attributes, fields), but with newtonsoft I need to declare my getters internal to not have a current schema with old config.)

New code and old code:

    public static void WriteSchemaToFile(string path)
    {
        // Create JSON schema
        var schemaBuilder = new JsonSchemaBuilder();
        var schema = schemaBuilder.FromType<ConfigFileJsonSchema>()
            .Title("PlexCleaner Configuration Schema")
            // TODO: .Version(new Uri("http://json-schema.org/draft-06/schema"))
            .Id(new Uri(SchemaUri))
            .Build();

        // Write to file
        File.WriteAllText(path, schema.ToString());
    }

/*
    public static void WriteSchemaToFile(string path)
    {
        // Create JSON schema
        var generator = new Newtonsoft.Json.Schema.Generation.JSchemaGenerator
        {
            // TODO: How to exclude Obsolete marked items?
            DefaultRequired = Newtonsoft.Json.Required.Default
        };
        var schema = generator.Generate(typeof(ConfigFileJsonSchema));
        schema.Title = "PlexCleaner Configuration Schema";
        schema.SchemaVersion = new Uri("http://json-schema.org/draft-06/schema");
        schema.Id = new Uri(SchemaUri);

        // Write to file
        File.WriteAllText(path, schema.ToString());
    }
*/

Problem is I use [Obsolete] and [Required] extensively, and now get this error:

E.g.

using System;
using System.ComponentModel;
using System.ComponentModel.DataAnnotations;
using System.IO;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Text.Json.Serialization.Metadata;
using Json.Schema;
using Json.Schema.Generation;
using Serilog;

public record ConfigFileJsonSchemaBase
{
    // Schema reference
    [Required]
    [JsonPropertyName("$schema")]
    [JsonPropertyOrder(-3)]
    public string Schema { get; } = SchemaUri;

    // Default to 0 if no value specified, and always write the version first
    [Required]
    [DefaultValue(0)]
    [JsonPropertyOrder(-2)]
    [JsonIgnore(Condition = JsonIgnoreCondition.Never)]
    public int SchemaVersion { get; set; } = ConfigFileJsonSchema.Version;
}
'Required' is an ambiguous reference between 'System.ComponentModel.DataAnnotations.RequiredAttribute' and 'Json.Schema.Generation.RequiredAttribute'

As the FromType seems like an extension, I could not find a way to be more explicit with the namespaces.

Any ideas how to resolve the ambiguity?

Code of Conduct

  • I agree to follow this project's Code of Conduct
@ptr727 ptr727 added the question Further information is requested label Apr 3, 2024
@gregsdennis
Copy link
Collaborator

gregsdennis commented Apr 3, 2024

You'll need to specify which one you mean using a using statement alias.

Either that or you can include the namespace with the attribute usage:

[System.ComponentModel.DataAnnotations.Required]

@ptr727
Copy link
Author

ptr727 commented Apr 3, 2024

I am familiar with aliases, I just failed to create one that worked, I also tried to be explicit with the json.schema.generation.jsonschemabuilder, but then the fromtype did not resolve.

I do not want to change all my existing [Required] annotations, or those in code I use but do not author, just because I included using Json.Schema, will experiment some more with aliases.

It does seem like a general problem when the schema package uses the same attribute as used in the standard dataannotations package though, anything you could do to not require all users to disambiguate, or document a solution that does not require me to change all my code?

@elgonzo
Copy link

elgonzo commented Apr 4, 2024

I am familiar with aliases, I just failed to create one that worked

I don't know what went wrong on your end, but using RequiredAttribute = Json.Schema.Generation.RequiredAttribute; should work just like that.

or document a solution that does not require me to change all my code?

Luckily, a solution for that is already documented, it's the global modifier, a feature of the C# language: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/using-directive#global-modifier. For example, add a new (and empty) .cs file to your project and put global using RequiredAttribute = Json.Schema.Generation.RequiredAttribute; into it. Et voilà!

@ptr727
Copy link
Author

ptr727 commented Apr 4, 2024

I think I misunderstood, I do want my existing [Required] attributes to come from System.ComponentModel.DataAnnotations, I do not want to replace them globally, or in code I use but do not author.

I am looking for an alias to apply only to to JsonSchemaBuilder and FromType, e.g.:

var schemaBuilder = new Json.Schema.JsonSchemaBuilder();
var schema = schemaBuilder.Json.Schema.Generation.FromType<Parent2>() /*this does not work for extension methods*/
    .Title("PlexCleaner Configuration Schema")
    .Id(new Uri(SchemaUri))
    .Build();

But, FromType() is an extension and I do not know how to namespace qualify that?

@ptr727 ptr727 closed this as completed Apr 4, 2024
@ptr727 ptr727 reopened this Apr 4, 2024
@elgonzo
Copy link

elgonzo commented Apr 4, 2024

I am looking for an alias to apply only to to JsonSchemaBuilder and FromType

That's not possible, because that's not how attribute annotations work in .NET. This is not even related to using aliases. It stems simply from the fact that attribute annotations, including the full type of the attributes represented by an attribute annotation are compiled as static meta data of the types or members the attribute annotations are applied to. This kind of meta data is not dynamic and is not able to morph. You can't have an alias to apply only to to JsonSchemaBuilder because that's not where the attribute annotations appear -- they appear in your types. Because of the meta-data being static, changing this meta data, i.e., changing the nature of the attribute annotation, would require a rebuild/recompilation of the types that are using the respective attribute annotations. (There might perhaps be some clever tricks when using dynamic IL code generation in your project, but that's both beyond my horizon and probably quite dirty and error-prone.)

@ptr727
Copy link
Author

ptr727 commented Apr 4, 2024

If I were to summarize, it is not possible to using System.ComponentModel.DataAnnotations; and using Json.Schema.Generation; and FromType<>() and [Required] at the same time.

@gregsdennis
Copy link
Collaborator

About a week ago, I released JsonSchema.Net.Generation.DataAnnotations, which provides support for the DataAnnotations namespace.

Announcement

Regarding wanting [Required] to be the DataAnnotations one, you can add a using or explicitly specify the namespace. You have to tell the compiler which one you want to use. There's not a way around that.

@ptr727
Copy link
Author

ptr727 commented Apr 4, 2024

About a week ago, I released JsonSchema.Net.Generation.DataAnnotations, which provides support for the DataAnnotations namespace.

Announcement

Docs?

Regarding wanting [Required] to be the DataAnnotations one, you can add a using or explicitly specify the namespace. You have to tell the compiler which one you want to use. There's not a way around that.

Than you for joining the conversation, I did cover this in my replies, I don't control all the code I use, and I don't want to be forced to disambiguate from a standard library when I include your code.

  1. you could have used a non-conflicting attribute in your code and it would not have been an issue, or 2) you could provide an alternate to FromType<>() that is strongly typed.

@gregsdennis
Copy link
Collaborator

Docs?

As stated in the follow-up tweet, I'm working on them.

I don't want to be forced to disambiguate from a standard library when I include your code.

The base generation library doesn't expect DataAnnotations to be present. Naming conflicts happen all of the time, even within the standard libraries.

you could have used a non-conflicting attribute in your code and it would not have been an issue

I'm sorry that you don't agree with my decision, but a very reasonable workaround exists.

you could provide an alternate to FromType<>() that is strongly typed.

That method is very strongly typed: you're passing the type in as the generic parameter. I'm not sure what you mean here.

@ptr727
Copy link
Author

ptr727 commented Apr 4, 2024

Ok, I feel we are getting of on the wrong foot, thank you for your work, it is just a bit difficult to add your library to code that kinda worked with Json.Net.

Asking me to look at announcement for details, and when I ask for docs say not ready as I said, is not really helping, I mean what did you want me to do?

I don't know what the very reasonable workaround is if I cannot change code I use that is using [Required] from DataAnnotations? (as I mentioned, not all my code, but yes I could add the namespace to all attributes)

In a previous comment I gave an actual code example of how I'd like to use but can't use FromType<>() because it is an extension / annotation that requires the namespace to be in scope (or whatever the right C# term is)

@gregsdennis
Copy link
Collaborator

gregsdennis commented Apr 4, 2024

it is just a bit difficult to add your library to code that kinda worked with Json.Net.

This suite of libraries uses System.Text.Json, which is a completely different system from Newtonsoft.Json. I expect you understand this. However because it's a different system, you should expect that there will be some transitioning and that not everything will work the same. Surely you've seen this just between the two JSON serializers.

The transition from Newtonsoft's schema generation to my schema generation should also carry the same expectation that things are going to work differently.

I don't know what the very reasonable workaround is if I cannot change code

Why can't you change the code? If you're having an ambiguity problem, then you have the code because it's being compiled. Is the code that carries the [Required] attribute generated?

In a previous comment I gave an actual code example of how I'd like to use but can't use FromType<>() because it is an extension / annotation that requires the namespace to be in scope (or whatever the right C# term is)

You don't have to call it as an extension method. Extension methods are just static methods, and you can call them that way. If you don't want to bring the namespace into scope, you can add the namespace to the extension class.

var schema = Json.Schema.Generation.JsonSchemaBuilderExtensions.FromType<MyClass>(builder).Build();

All of the builder methods are extension methods, actually.

@ptr727
Copy link
Author

ptr727 commented Apr 4, 2024

Calling as static will be a solution as I can avoid needing to use using ....

This requires using ... but works:

var schemaBuilder = new JsonSchemaBuilder();
var schema = schemaBuilder.FromType<Parent2>()
    .Title("PlexCleaner Configuration Schema")
    .Id(new Uri(SchemaUri))
    .Build();
var jsonSchema = JsonSerializer.Serialize(schema);

This should not require using ... but does mot resolve .Title()?

const string SchemaUri = "https://raw.githubusercontent.com/ptr727/PlexCleaner/main/PlexCleaner.schema.json";
var schemaBuilder = new Json.Schema.JsonSchemaBuilder();
var schema = Json.Schema.Generation.JsonSchemaBuilderExtensions.FromType<Parent2>(schemaBuilder)
    .Title("PlexCleaner Configuration Schema") // Does not resolve?
    .Id(new Uri(SchemaUri))
    .Build();
var jsonSchema = JsonSerializer.Serialize(schema);

My C# foo is not strong enough, advice on how to make this work with full namespaces specified?

@gregsdennis
Copy link
Collaborator

gregsdennis commented Apr 4, 2024

You should still be able to include the normal builder extensions:

using Json.Schema;

var builder = new JsonSchemaBuilder()
    .Title("PlexCleaner Configuration Schema")
    .Id(new Uri(SchemaUri));
var schema = Json.Schema.Generation.JsonSchemaBuilderExtensions.FromType<Parent2>(builder)
    .Build();

Only .FromType() and the generation attributes are defined in the .Generation namespace.

@ptr727
Copy link
Author

ptr727 commented Apr 4, 2024

Thank you, that works great.

If I want to be pedantic I can also use Json.Schema.JsonSchemaBuilderExtensions.Title(builder, "PlexCleaner Configuration Schema");

@gregsdennis
Copy link
Collaborator

It's the same way. The extension class is Json.Schema.JsonSchemaBuilderExtensions.

Out of curiousity, why are you so opposed to adding using statements? You obviously have the code (because it's not compiling), so why not change it? using statements are normal C# features and should be utilized.

@ptr727
Copy link
Author

ptr727 commented Apr 4, 2024

Oops, sorry, figured that out right after I commented.

It is not that I am opposed to not using using it is that adding the using Json had unexpected repercussions.
Now I do hear you conflicts do arise, but when the conflict is a 3rd party lib with a .NET standard lib, my inclination is to ask the 3rd party lib to stop conflicting with the standard.

In my open source projects yeah I can just go and change all my [Required] tags, or get rid of them, but in consulting / team projects introducing this lib as a plays nice with Text.Json substitute for Json.Net, and then making other people change their code, or getting buy in to change their code, will have the same "but why" experience as me asking you but why did you choose to use a conflicting attribute.

@gregsdennis
Copy link
Collaborator

Okay, so here's my explanation:

The attributes in System.ComponentModel.DataAnnotations have no effect on System.Text.Json's serialization. The two namespaces are completely disjoint. DataAnnotations is used to annotate ASP.Net DTOs and provide validation constraints.

I actually go over some of this in a blog post. Essentially, the model is deserialized (ignoring the DataAnnotations attributes), then the model is validated against the attributes in a separate operation.

When I was designing schema generation, my focus was System.Text.Json. Since the data annotations and serialization do not interact, there was no reason to consider them. I only output my own DataAnnotations extension package recently due to clients requesting it, and then reluctantly so.

Regarding using statements, the alias usage has been a staple in C# for a very long time, and no one should be surprised by them, especially professional teams.

I have no intention of renaming my attribute. It's aptly named, and in the rare event conflicts arise, the language has the mechanisms needed to resolve the ambiguity.

@ptr727
Copy link
Author

ptr727 commented Apr 4, 2024

Understood, for reference, some projects I work on do use the same code in DTO's in DB's and JSON serialization, thus the potential conflict, anyway I now have ways of not using using at all if need be.

Side questions, could not find in docs, what is the correct $schema for the generated content, e.g. https://json-schema.org/draft/2020-12/schema, http://json-schema.org/draft-06/schema, etc.?

@gregsdennis
Copy link
Collaborator

Theoretically, I should be including it. It's nominally 2020-12, but pretty much everything should be compatible with all of them.

@ptr727
Copy link
Author

ptr727 commented Apr 4, 2024

Theoretically, I should be including it.

Not in my testing?

@gregsdennis
Copy link
Collaborator

gregsdennis commented Apr 4, 2024

I don't understand your question. I mean that I should be including a $schema in my generation (but I don't), however due to the compatability it doesn't matter.

@ptr727
Copy link
Author

ptr727 commented Apr 26, 2024

In my testing $schema is not included by default, and has to be explicitly added.

Closing as the advice I got here let me resolve the namespace issue.

@ptr727 ptr727 closed this as completed Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:schema-generation question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants