Feature/issue-40 #41

dnllowe · 2019-06-17T00:53:01Z

My team and I have been using this library and it's made life much easier when working with quads and the rdf format. Thanks!

However, we have a need to return jsonld where order matters. But, the JsonLdApi.FromRDF method sorts properties by default. We need to be able to control when to sort the top-level graphs, the child nodes, or both.

The new SortingTests provide examples for what we're after, but as a summary, given the json below, we'd want to control whether nodes 1, 2, and 3 get sorted, and whether their children (objects 1, 2, and 3) are sorted. As of now, the JsonLdApi.FromRDF will always sort the graph nodes and children, losing the orignal order.

[
    {
        "@id": "http://example.org/node/3",
        "@graph": [
            {
                "@id": "http://example.org/object/3",
                "http://example.org/value": [
                    {
                        "@id": "n3-o3-value"
                    }
                ]
            },
            {
                "@id": "http://example.org/object/1",
                "http://example.org/value": [
                    {
                        "@id": "n3-o1-value"
                    }
                ]
            },
            {
                "@id": "http://example.org/object/2",
                "http://example.org/value": [
                    {
                        "@id": "n3-o2-value"
                    }
                ]
            }
        ]
    },
    {
        "@id": "http://example.org/node/1",
        "@graph": [
            {
                "@id": "http://example.org/object/3",
                "http://example.org/value": [
                    {
                        "@id": "n1-o3-value"
                    }
                ]
            },
            {
                "@id": "http://example.org/object/1",
                "http://example.org/value": [
                    {
                        "@id": "n1-o1-value"
                    }
                ]
            },
            {
                "@id": "http://example.org/object/2",
                "http://example.org/value": [
                    {
                        "@id": "n1-o2-value"
                    }
                ]
            }
        ]
    },
    {
        "@id": "http://example.org/node/2",
        "@graph": [
            {
                "@id": "http://example.org/object/3",
                "http://example.org/value": [
                    {
                        "@id": "n2-o3-value"
                    }
                ]
            },
            {
                "@id": "http://example.org/object/1",
                "http://example.org/value": [
                    {
                        "@id": "n2-o1-value"
                    }
                ]
            },
            {
                "@id": "http://example.org/object/2",
                "http://example.org/value": [
                    {
                        "@id": "n2-o2-value"
                    }
                ]
            }
        ]
    }
]

I've gotten this to work only with JsonLdApi.FromRDF, but that's the only need we have, so I've left the rest of the code alone as much as possible. Testing the same changes with Compact revealed just how complicated it might be to try and control the order in all cases--but if the original input for the other methods (Compact, Flatten, Expand, etc) is in the order that's desired, then it seems to remain unchanged. It was only FromRDF the changed the order of the input.

Fixes #40.

…der instead

dnllowe · 2019-06-17T00:55:51Z

src/json-ld.net/Core/JsonLdOptions.cs

@@ -43,6 +42,9 @@ public virtual JsonLD.Core.JsonLdOptions Clone()

        private bool produceGeneralizedRdf = false;

+        private bool sortGraphsFromRdf = true;


Since I've only been able to control the order with the FromRDF method (which luckily is all that's needed to get the desired behavior), I made the option pretty narrow in scope.

dnllowe · 2019-06-17T00:56:32Z

test/json-ld.net.tests/ConformanceTests.cs

@@ -16,7 +13,7 @@ namespace JsonLD.Test
    public class ConformanceTests
    {
        [Theory, ClassData(typeof(ConformanceCases))]
-        public void ConformanceTestPasses(string id, string testname, ConformanceCase conformanceCase)
+        public void ConformanceTestPasses(string id, ConformanceCase conformanceCase)


The testname variable was never used, so removing it had no impact

dnllowe · 2019-06-17T00:56:53Z

test/json-ld.net.tests/ConformanceTests.cs

@@ -77,20 +74,21 @@ public ConformanceCases()

        public IEnumerator<object[]> GetEnumerator()
        {
+            var jsonFetcher = new JsonFetcher();


I created the JsonFetcher class so I could reuse some of this logic in my own test class

codecov-io · 2019-06-17T00:57:25Z

Codecov Report

Merging #41 into master will increase coverage by 0.20%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #41      +/-   ##
==========================================
+ Coverage   68.38%   68.59%   +0.20%     
==========================================
  Files          22       22              
  Lines        4043     4053      +10     
  Branches     1026     1028       +2     
==========================================
+ Hits         2765     2780      +15     
+ Misses       1133     1130       -3     
+ Partials      145      143       -2

Impacted Files	Coverage Δ
src/json-ld.net/Core/JsonLdProcessor.cs	`68.68% <ø> (ø)`
src/json-ld.net/Core/NormalizeUtils.cs	`95.47% <ø> (ø)`
src/json-ld.net/Core/JsonLdApi.cs	`84.85% <100.00%> (+0.03%)`	⬆️
src/json-ld.net/Core/JsonLdOptions.cs	`81.63% <100.00%> (+3.58%)`	⬆️
src/json-ld.net/Core/JsonLdUtils.cs	`27.98% <0.00%> (+0.72%)`	⬆️
src/json-ld.net/Core/RDFDataset.cs	`69.54% <0.00%> (+0.75%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5114477...025f592. Read the comment docs.

dnllowe · 2019-06-17T00:57:38Z

test/json-ld.net.tests/ConformanceTests.cs

                }
            }
        }

-        private JToken GetJson(JToken j)


Moved to the JsonFetcher class

dnllowe · 2019-06-17T00:59:18Z

test/json-ld.net.tests/Sorting/W3C/compact-in.jsonld

@@ -0,0 +1,89 @@
+[


After getting FromRDF to work, I tried the same approach with Compact, but it seemed too complicated to get it to work. Plus, if Compact or the other methods are given input, they seem to maintain the order. I left the remnants of the test here though in case it seemed worthwhile to try again later.

Or we can just delete it.

dnllowe · 2019-06-17T01:00:08Z

test/json-ld.net.tests/Sorting/W3C/fromRdf-in.json

@@ -0,0 +1,58 @@
+{


This was the easiest way for me to create quads to provide the FromRDF method.

dnllowe · 2019-06-17T01:01:57Z

test/json-ld.net.tests/SortingTests.cs

@@ -0,0 +1,143 @@
+using JsonLD.Core;


I wanted to keep the sorting tests self-contained and not mix them in with the other tests, assuming that if the other tests are serializing correctly, and this is sorting correctly, then all is well.

goofballLogic · 2019-06-26T18:25:13Z

It's important to maintain the same API as that exposed by jsonld.js can you check what they are doing on this issue?

asbjornu · 2020-05-04T15:15:30Z

@dnllowe, do you think we could move this PR forward by answering @goofballLogic's question? Looks like @xivk's review would be appreciated as well.

dnllowe · 2020-05-15T09:12:22Z

@dnllowe, do you think we could move this PR forward by answering @goofballLogic's question? Looks like @xivk's review would be appreciated as well.

Sorry--the company had ended up forking the repository for the change. But let me take a look this weekend and get back to you!

dnllowe · 2020-05-16T15:50:50Z

The jsonld.js library also sorts by default, with no option to preserve the sorting order. I've opened an issue/PR for the library to allow the same behavior I've proposed here:

digitalbazaar/jsonld.js#396

goofballLogic · 2020-05-20T17:10:07Z

sounds like the jsonld guys are on the fence about this functionality. Is there a way we could distinguish between "conformance" tests and "extra functionality" tests in our suite - so that there's a clear distinction between the official suite's tests and this extra function?

dnllowe · 2020-05-23T11:21:54Z

sounds like the jsonld guys are on the fence about this functionality. Is there a way we could distinguish between "conformance" tests and "extra functionality" tests in our suite - so that there's a clear distinction between the official suite's tests and this extra function?

That sounds reasonable. Could go with either CustomOrderingTests to keep it specific and isolated, or something like ExtendedFunctionalityTests if you want a destination for any possible diverging functionality in the future.

goofballLogic · 2020-05-31T12:11:18Z

ExtendedFunctionalityTests sounds good. Apologies for the lax response on this - challenging times for everyone at the moment I think!

dnllowe · 2020-06-01T23:06:16Z

ExtendedFunctionalityTests sounds good. Apologies for the lax response on this - challenging times for everyone at the moment I think!

Hey, no worries at all-and no rush! You've already been a great help, and I appreciate it :)

Moves to ExtendedFunctionalityTests Refactors to create room for more extended functionality tests

goofballLogic

Seems sound to me

asbjornu · 2020-06-25T09:56:44Z

Is this a breaking change so we should consider bumping the major version after merging this?

goofballLogic · 2020-06-25T10:03:12Z

Is this a breaking change so we should consider bumping the major version after merging this?

I think this is adding a predictable behaviour (sorting) where previously the behaviour was, in theory, unpredictable. Is it possible that somebody could depend on things not being sorted???

asbjornu · 2020-06-25T11:08:34Z

I agree that sounds highly unlikely, @goofballLogic. Let's merge and bump minor.

dnllowe added 5 commits June 14, 2019 23:10

Makes lexicographical sorting optional, preserving original object or…

a04af73

…der instead

Adds tests for JsonLdApi.FromRDF

058d7e1

Limits ordering logic to JsonLdApi.FromRdf

743598f

Removes line spaces that were added

af60610

Updates to csproj version

4777911

dnllowe commented Jun 17, 2019

View reviewed changes

test/json-ld.net.tests/ConformanceTests.cs

}

}

}

private JToken GetJson(JToken j)

Copy link

Contributor Author

dnllowe Jun 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to the JsonFetcher class

dnllowe commented Jun 17, 2019

View reviewed changes

dnllowe added 2 commits June 16, 2019 21:04

Rename test case

4b310d5

Removes leftover test code

5a5662f

goofballLogic requested a review from xivk August 21, 2019 11:48

Merge branch 'master' into feature/issue-40

3d1f035

goofballLogic mentioned this pull request May 20, 2020

Control order of graph nodes and children nodes fromRdf digitalbazaar/jsonld.js#395

Open

Merge branch 'master' into feature/issue-40

858ef20

dnllowe added 2 commits June 1, 2020 23:19

Updates test

69820b3

Moves to ExtendedFunctionalityTests Refactors to create room for more extended functionality tests

Some cleanup

0e35a8f

goofballLogic approved these changes Jun 4, 2020

View reviewed changes

Merge branch 'master' into feature/issue-40

1e8ebbd

Merge branch 'master' into feature/issue-40

025f592

asbjornu merged commit 7202bc6 into linked-data-dotnet:master Jun 25, 2020

dnllowe deleted the feature/issue-40 branch June 30, 2020 03:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/issue-40 #41

Feature/issue-40 #41

dnllowe commented Jun 17, 2019 •

edited by asbjornu

Loading

dnllowe Jun 17, 2019

dnllowe Jun 17, 2019

dnllowe Jun 17, 2019

codecov-io commented Jun 17, 2019 •

edited by codecov bot

Loading

dnllowe Jun 17, 2019

dnllowe Jun 17, 2019

dnllowe Jun 17, 2019

dnllowe Jun 17, 2019

goofballLogic commented Jun 26, 2019

asbjornu commented May 4, 2020

dnllowe commented May 15, 2020

dnllowe commented May 16, 2020 •

edited

Loading

goofballLogic commented May 20, 2020

dnllowe commented May 23, 2020

goofballLogic commented May 31, 2020

dnllowe commented Jun 1, 2020

goofballLogic left a comment

asbjornu commented Jun 25, 2020

goofballLogic commented Jun 25, 2020 •

edited

Loading

asbjornu commented Jun 25, 2020

		@@ -43,6 +42,9 @@ public virtual JsonLD.Core.JsonLdOptions Clone()

		private bool produceGeneralizedRdf = false;

		private bool sortGraphsFromRdf = true;

Feature/issue-40 #41

Feature/issue-40 #41

Conversation

dnllowe commented Jun 17, 2019 • edited by asbjornu Loading

dnllowe Jun 17, 2019

Choose a reason for hiding this comment

dnllowe Jun 17, 2019

Choose a reason for hiding this comment

dnllowe Jun 17, 2019

Choose a reason for hiding this comment

codecov-io commented Jun 17, 2019 • edited by codecov bot Loading

Codecov Report

dnllowe Jun 17, 2019

Choose a reason for hiding this comment

dnllowe Jun 17, 2019

Choose a reason for hiding this comment

dnllowe Jun 17, 2019

Choose a reason for hiding this comment

dnllowe Jun 17, 2019

Choose a reason for hiding this comment

goofballLogic commented Jun 26, 2019

asbjornu commented May 4, 2020

dnllowe commented May 15, 2020

dnllowe commented May 16, 2020 • edited Loading

goofballLogic commented May 20, 2020

dnllowe commented May 23, 2020

goofballLogic commented May 31, 2020

dnllowe commented Jun 1, 2020

goofballLogic left a comment

Choose a reason for hiding this comment

asbjornu commented Jun 25, 2020

goofballLogic commented Jun 25, 2020 • edited Loading

asbjornu commented Jun 25, 2020

dnllowe commented Jun 17, 2019 •

edited by asbjornu

Loading

codecov-io commented Jun 17, 2019 •

edited by codecov bot

Loading

dnllowe commented May 16, 2020 •

edited

Loading

goofballLogic commented Jun 25, 2020 •

edited

Loading