Enable checking against a schema #2027

Olympic1 · 2017-04-03T05:28:03Z

From KSP-CKAN/CKAN-core#133

Rebased PR #992

politas · 2017-04-03T08:27:37Z

@dbent , you're the Cake expert. Can you figure out what's happening here? These .csproj files baffle me.

dbent · 2017-04-03T13:56:52Z

Well we still need to consider if we want to do this. Json.NET Schema is not permissively licensed, but is licensed under AGPL-3.0 which is copyleft. This means the combined work (ckan.exe, netkan.exe, etc.) has to be licensed under the AGPL. Our source code is MIT licensed which is (A)GPL compatible so that doesn't have to change though.

There's a few things going on in this PR with regards to the build, specifically updating versions of libraries. I'll make a separate PR for that.

Olympic1 · 2017-04-03T15:07:41Z

I already thought that it had to do something with the lib updates.

politas · 2017-04-03T20:02:53Z

That doesn't take away the MIT license from the CKAN code, does it? This whole question of multiple licenses gets pretty confusing. Would it mean that someone could fork CKAN and claim only the AGPL applies to their new fork?

Actually, aren't we already including some copyleft packages?

dbent · 2017-04-04T15:22:55Z

IANAL, but here's my understanding:

That doesn't take away the MIT license from the CKAN code, does it?

No, MIT aka Expat license, is "GPL-Compatible". GPL-Compatible source can be combined with (A)GPL source to create a derived work that doesn't force the GPL-Compatible source to be released under the (A)GPL. The derived work (ckan.exe, netkan.exe, etc) however must be.

What this means, is that if someone wants to use ckan.exe or netkan.exe as a library (which is totally possible in .NET), their work would have to released under the (A)GPL. To avoid that, they'd have to take our MIT source, excise any bits that refer to (A)GPL code and create a custom build derived only from MIT source. (This could be automated through the use of #ifdef in the source and a bit smarter build logic to avoid ILRepacking Json.NET Schema). What's more, the AGPL (and its primary difference with respect to the GPL) is that even use of an application over a network is considered distribution and thus triggers its provisions. So a web application using ckan.exe/netkan.exe would have to be distributed under AGPL.

Now, this person might also be able to avoid that by calling ckan.exe/netkan.exe as a separate process, but if the program is "specifically designed to require" ckan.exe/netkan.exe then the provisions probably apply anyway.

Would it mean that someone could fork CKAN and claim only the AGPL applies to their new fork?

No, but it probably means that different source files would have to be under different licenses. Anything that uses JSON.NET Schema would have to be relicensed under AGPL (and relicensing requires consent of all contributors to that file, since CKAN has no copyright assignment to a single individual/organization so every author owns the copyright to their own contributions).

Now that I think about, even excising code could be problematic. Since if the entire file is AGPL, then removing code just makes a new derived work that would also have to be AGPL. Can copyright be specified on a line-by-line basis? Hmmm.

Actually, aren't we already including some copyleft packages?

Nope. I went through each and every library we use, including ones only used for testing and all except one are licensed under permissive (non-copyleft) MIT, Apache, or BSD licenses. The only one that isn't licensed under a permissive license is ICSharpCode.SharpZipLib.Patched but it's licensed under the GPL with an explicit linking exception making it similar to the LGPL.

Autofac: MIT (permissive)
Castle.Core: Apache-2.0 (permissive)
ChinhDo.Transactions.FileManager: MIT (permissive)
CommandLineParser: MIT (permissive)
ICSharpCode.SharpZipLib.Patched: GPL w/ Linking Exception (semi-permissive)
ini-parser: MIT (permissive)
log4net: Apache-2.0 (permissive)
Moq: BSD 3 Clause (permissive)
Newtonsoft.Json: MIT (permissive)
NUnit: MIT (permissive)
TxFileManager: MIT (permissive)
CurlSharp: BSD 3 Clause (permissive)

The whole issue with the AGPL is a minefield of technicalities, which is why I've never bothered to incorporate JSON.NET Schema into NetKAN's validation.

FYI I've made #2028 which updates the build and all our libraries to the latest and greatest. If we want to pursue this, it should be probably be based off that.

dbent · 2017-04-04T15:29:57Z

Also, the AGPL version of JSON.NET Schema is listed as "Free with limitations (1000 validations per hour)". I'm not entirely sure what that means.

Olympic1 · 2017-04-04T16:39:31Z

Well, in that case it would be better to find another way to check the spec or just drop it.

techman83 · 2017-04-04T23:26:01Z

To note, both the indexer and Jenkins run the resulting output through a schema validator. So what makes it CKAN-meta has been validated.

It would probably be handy to have in the client for those that make custom ckans or provide early signs that a netkan will fail CI, but overall I don't think the need is exceptionally high. A nice to have for sure, but it's not really a big deal in the scheme of things.

dbent · 2017-04-05T14:22:01Z

NJsonSchema is another project that seems active and is permissively (MIT) licensed.

Dazpoet · 2017-04-20T20:55:23Z

Don't we have someone cough @pjf cough who loves discussing licenses and how they work and not work?

techman83 · 2017-04-21T01:01:07Z

@Dazpoet I think it's more @pjf is exceptionally knowledgeable on the topic and has a passion for ensuring it's done correctly. If there is an MIT based plugin lets use it. Also the restriction added by JSON.NET schema may not be applicable:

https://www.gnu.org/licenses/agpl-3.0.en.html

All other non-permissive additional terms are considered "further restrictions" within the meaning of section 10. If the Program as you received it, or any part of it, contains a notice stating that it is governed by this License along with a term that is a further restriction, you may remove that term.

Also AGPL is quite at the opposite end of the spectrum when compared to MIT, I'm not super across how it would effect things when used as a library though and we'd want to be very sure we're across the implications. It would certainly effect the ability for it's use in a commercial product, so if there is an MIT licensed project available, lets use that. But we're not in a rush for such a feature and we'd need to do some indexer work if NetKAN started throwing it's own schema exceptions.

ayan4m1 · 2017-09-28T21:17:26Z

@Olympic1 Can you resolve conflicts/rebase or close this out?

politas · 2017-10-15T22:31:06Z

@Olympic1, can you have a look at NJsonSchema that @dbent mentioned and see if switching to that is possible?

RicoSuter · 2017-10-16T23:00:41Z

Hi everybody

Im the creator of NJsonSchema.

Switching should be very simple, only problem i see in the schema is the “required” properties which have no corresponding properties. We have an open issue for that.

Other than that just load the schema with JsonSchema4.FromJsonAsync() and call Validate().

Olympic1 · 2017-11-11T00:59:28Z

@dbent, @politas, I converted the PR to NJsonSchema, but not fully.

I followed @RSuter's instructions and fixed the errors. Need to look into some more.

Olympic1 · 2017-11-22T00:57:32Z

Rebased. Have anyone had the time to test this PR? I want to move this forward.

HebaruSan

I'm a little anxious about the Validate call, but maybe I'm misinterpreting something.

HebaruSan · 2017-11-22T16:21:05Z

Core/Types/CkanModule.cs

+ }
+ }
+
+ private static readonly string SchemaPath = "CKAN.CKAN.schema";


Let's make this a const instead of static readonly, since the value is known at compile time.

HebaruSan · 2017-11-22T16:32:00Z

Core/Types/CkanModule.cs

+ JsonSchema4 schema = await JsonSchema4.FromJsonAsync(json);
+
+ IList<string> errorList = new List<string>();
+ var errors = schema.Validate(MetadataSchema.ToJson());


If I'm reading this correctly, we process the schema from a JSON string to a JsonSchema4 object in the static constructor, and then back from object to a JSON string in the validation function.

Can we store the return value of the ToJson function, so we don't have to pay the cost of that conversion for every CkanModule object? Presumably we can ensure that the CKAN.schema file bundled in the assembly is valid separately whenever we modify it.

HebaruSan · 2017-11-22T16:37:21Z

Core/Types/CkanModule.cs

+ if (errors.Result.Any())
+ {
+ var message = "Validation against the schema failed.\n" + string.Join("\n ", errors);
+ throw new BadMetadataKraken(null, message);


I'd like to test the impact of this exception on the GUI. What would be a good way to do that? I tried corrupting my registry.json file in various ways, but I think those tests got caught by other checks (e.g., invalid licenses are already covered elsewhere).

HebaruSan · 2017-11-22T16:39:07Z

Core/CKAN.schema

@@ -0,0 +1,423 @@
+{


Is it impossible to move the old CKAN.schema file to this new location? It will be inconvenient to have to maintain two files identically going forward.

It could be a symbolic link instead of a separate file, surely?

HebaruSan · 2017-11-22T16:51:49Z

Core/Types/CkanModule.cs

+
+ private static async Task<IList<string>> ValidateAgainstSchema(string json)
+ {
+ JsonSchema4 schema = await JsonSchema4.FromJsonAsync(json);


This variable represents the module we're validating, not the schema, right? Let's change its name to clarify that.

HebaruSan · 2017-11-22T16:53:08Z

Core/Types/CkanModule.cs

+ JsonSchema4 schema = await JsonSchema4.FromJsonAsync(json);
+
+ IList<string> errorList = new List<string>();
+ var errors = schema.Validate(MetadataSchema.ToJson());


Actually, are you sure this call is correct? The schema object appears to be the module, and the parameter with ToJson is the schema. This looks like we're trying to use the module to validate the schema rather than the other way around. The documentation passes the data to be validated to the schema's Validate function.

Olympic1 · 2019-07-21T15:54:01Z

Closing as this was done in #2788

Olympic1 added the Pull request label Apr 3, 2017

Olympic1 mentioned this pull request Apr 3, 2017

Enable checking against a schema. #992

Closed

Olympic1 added Core (ckan.dll) Issues affecting the core part of CKAN Enhancement Schema Issues affecting the schema labels Apr 3, 2017

Olympic1 requested review from dbent and politas November 11, 2017 01:32

Olympic1 self-assigned this Nov 11, 2017

Enable checking against a schema.

8680a5a

Olympic1 force-pushed the EnableSchema branch from 0258d52 to 8680a5a Compare November 22, 2017 00:49

HebaruSan requested changes Nov 22, 2017

View reviewed changes

Olympic1 mentioned this pull request Jun 19, 2019

Migrate Perl validation checks into netkan.exe #2788

Merged

DasSkelett mentioned this pull request Jun 24, 2019

[Feature] Refactor CKAN-core to use .NET standard 2.0 #2799

Closed

Olympic1 closed this Jul 21, 2019

Olympic1 deleted the EnableSchema branch July 29, 2020 07:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable checking against a schema #2027

Enable checking against a schema #2027

Olympic1 commented Apr 3, 2017 •

edited

Loading

politas commented Apr 3, 2017

dbent commented Apr 3, 2017

Olympic1 commented Apr 3, 2017

politas commented Apr 3, 2017

dbent commented Apr 4, 2017 •

edited

Loading

dbent commented Apr 4, 2017

Olympic1 commented Apr 4, 2017

techman83 commented Apr 4, 2017

dbent commented Apr 5, 2017

Dazpoet commented Apr 20, 2017

techman83 commented Apr 21, 2017

ayan4m1 commented Sep 28, 2017

politas commented Oct 15, 2017

RicoSuter commented Oct 16, 2017

Olympic1 commented Nov 11, 2017

Olympic1 commented Nov 22, 2017

HebaruSan left a comment

HebaruSan Nov 22, 2017

HebaruSan Nov 22, 2017

HebaruSan Nov 22, 2017

HebaruSan Nov 22, 2017

politas Nov 28, 2017

HebaruSan Nov 22, 2017

HebaruSan Nov 22, 2017

Olympic1 commented Jul 21, 2019

Enable checking against a schema #2027

Enable checking against a schema #2027

Conversation

Olympic1 commented Apr 3, 2017 • edited Loading

politas commented Apr 3, 2017

dbent commented Apr 3, 2017

Olympic1 commented Apr 3, 2017

politas commented Apr 3, 2017

dbent commented Apr 4, 2017 • edited Loading

dbent commented Apr 4, 2017

Olympic1 commented Apr 4, 2017

techman83 commented Apr 4, 2017

dbent commented Apr 5, 2017

Dazpoet commented Apr 20, 2017

techman83 commented Apr 21, 2017

ayan4m1 commented Sep 28, 2017

politas commented Oct 15, 2017

RicoSuter commented Oct 16, 2017

Olympic1 commented Nov 11, 2017

Olympic1 commented Nov 22, 2017

HebaruSan left a comment

Choose a reason for hiding this comment

HebaruSan Nov 22, 2017

Choose a reason for hiding this comment

HebaruSan Nov 22, 2017

Choose a reason for hiding this comment

HebaruSan Nov 22, 2017

Choose a reason for hiding this comment

HebaruSan Nov 22, 2017

Choose a reason for hiding this comment

politas Nov 28, 2017

Choose a reason for hiding this comment

HebaruSan Nov 22, 2017

Choose a reason for hiding this comment

HebaruSan Nov 22, 2017

Choose a reason for hiding this comment

Olympic1 commented Jul 21, 2019

Olympic1 commented Apr 3, 2017 •

edited

Loading

dbent commented Apr 4, 2017 •

edited

Loading