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

Field sequence definitions of schema #300

Open
kunlinyu opened this issue Oct 12, 2022 · 4 comments
Open

Field sequence definitions of schema #300

kunlinyu opened this issue Oct 12, 2022 · 4 comments

Comments

@kunlinyu
Copy link
Contributor

kunlinyu commented Oct 12, 2022

I defined a JSON file format. And generate schema by Newtonsoft.Json.Schema.

{
  "key1": "value1",
  "key2":"value2",
}

In case, I modify the C# code, which may change the schema, I will calculate the md5sum check sum for the schema for each version, and record it into file format.

class Data
{
  int key1;
  int key2;
  string schemaMd5;
  string ToJson()
  {
    JSchema schema = new JSchemaGenerator().Generate(typeof(Data));
    this.schemaMd5 = MD5SUM(schema.ToString());
    return JsonConvert.SerializeObject(this);
  }
}
{
  "key1": "value1",
  "key2":"value2",
  "schemaMd5":"1234567890ABCDEF"
}

Then future version may read the check sum to know the version of file format.
But I found that the JSchema.ToString() is not stable.
The order of field may change. So the schemaMd5 may change even no C# code changed.

I check the code:
https://github.com/JamesNK/Newtonsoft.Json.Schema/blob/master/Src/Newtonsoft.Json.Schema/Infrastructure/Collections/DictionaryBase.cs#L61
The JSchemaDictionary inherit from DictionaryBase which use Dictionary to be the default container.
I think if we use SortedDictionary(https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.sorteddictionary-2?view=net-6.0) then the output of JSchema.ToString() may be stable.

So my question is:
How can I use SortedDictionary to be the container of DictionaryBase?
Can we use SortedDictionary to be the default container of DictionaryBase?

@kunlinyu
Copy link
Contributor Author

kunlinyu commented Oct 13, 2022

After I set SchemaPropertyOrderHandling to Alphabetical, I think the JSchema.ToString() is stable.
But if I leave SchemaPropertyOrderHandling to Default, it is unstable.

JSchema schema = new JSchemaGenerator(){ SchemaPropertyOrderHandling = SchemaPropertyOrderHandling.Alphabetical }.Generate(typeof(Data));

@kunlinyu
Copy link
Contributor Author

I think we should set SchemaLocationHandling to "Inline" instead of Definitions.

Because according to:
https://github.com/JamesNK/Newtonsoft.Json.Schema/blob/master/Src/Newtonsoft.Json.Schema/Generation/JSchemaGeneratorInternal.cs#L592
definitions will not sort by SchemaPropertyOrderHandling.Alphabetical.

And this is just a workarround.
I think SchemaPropertyOrderHandling.Alphabetical should sort definitions.

@kunlinyu
Copy link
Contributor Author

https://github.com/JamesNK/Newtonsoft.Json.Schema/blob/master/Src/Newtonsoft.Json.Schema/Generation/JSchemaGeneratorInternal.cs#L76
If I change this line:

foreach (KeyValuePair<string, JSchema> definitionSchema in definitionsSchemas.OrderBy(s => s.Key))

to

foreach (KeyValuePair<string, JSchema> definitionSchema in definitionsSchemas.OrderBy(s => s.Key, StringComparer.Ordinal))

The bug gone.
But I am not sure if it is the correct solution.

So I replace Dictionary by SortedDictionary(xxx, StringComparer.Ordinal) as much as possible.
Such as
https://github.com/JamesNK/Newtonsoft.Json.Schema/blob/master/Src/Newtonsoft.Json.Schema/Generation/JSchemaGeneratorInternal.cs#L45
and
https://github.com/JamesNK/Newtonsoft.Json.Schema/blob/master/Src/Newtonsoft.Json.Schema/JSchema.cs#L55
and
https://github.com/JamesNK/Newtonsoft.Json.Schema/blob/master/Src/Newtonsoft.Json.Schema/JSchema.cs#L558

@kunlinyu kunlinyu changed the title Field sequence of schema Field sequence definitions of schema Oct 13, 2022
@kunlinyu
Copy link
Contributor Author

Screenshot from 2022-10-14 21-53-02

I finish more tests.
The result is:

  1. For the webGL build, I can not change the order of "definitions" of schema by set the comparer of OrderBy or leave it default.
  2. For the PC build (no matter mono backend or IL2CPP backend), the order of "definitions" can be changed by set the comparer.
  3. the default comparer of PC build is "OrdinalIgnoreCase"

So, the conclusion:
If we set the comparer of OrderBy to "Ordinal", then the schema is stable for all platform.

kunlinyu added a commit to kunlinyu/Newtonsoft.Json.Schema that referenced this issue Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant