Nested sort #416

milosloub · 2018-09-30T20:05:33Z

Closes #359

There is new approach of handling nested sort with refactor through all necessary layers.
I tried my best to keep compatibility with current implementations. Also FilterOperations addition to eliminate string typed operation parameter.

…s replaced by <=and vice versa)

…thods

…back to provide compatibility.

milosloub · 2018-09-30T20:10:17Z

src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs

            return sortQuery.Direction == SortDirection.Descending
                ? source.ThenByDescending(sortQuery.SortedAttribute.InternalAttributeName)
                : source.ThenBy(sortQuery.SortedAttribute.InternalAttributeName);
        }

-        public static IOrderedQueryable<TSource> OrderBy<TSource>(this IQueryable<TSource> source, string propertyName)
+        public static IQueryable<TSource> Sort<TSource>(this IQueryable<TSource> source, IJsonApiContext jsonApiContext, List<SortQuery> sortQueries)


Because of backward compatibility, I have to copy all three Sort methods with IJsonApiContext parameter.
It's important to distinguish whether the client use SortQuery with "old" SortedAttribute or new string based Attribute parameter. SortedAttribute depends on IJsonApiContext and string based Attribute not, so that's why this injection.

milosloub · 2018-09-30T20:15:31Z

src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs

-            var lambda = Expression.Lambda(property, parameter);
+            MemberExpression member;
+
+            var values = propertyName.Split('.');


To keep OrderBy, OrderByDescending, ThenBy and ThenByDescending method parameters without change, it's necessary to provide propertyName as path( "prop" or "rel.prop").

milosloub · 2018-09-30T20:16:54Z

src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs

-        }
-
-        public static IQueryable<TSource> Filter<TSource>(this IQueryable<TSource> source, RelatedAttrFilterQuery filterQuery)
+        public static IQueryable<TSource> Filter<TSource>(this IQueryable<TSource> source, BaseFilterQuery filterQuery)


BaseFilterQuery here as ancestor of AttrFilterQuery and RelatedAttrFilterQuery, so all filtering can be handled in one method due to DRY

milosloub · 2018-09-30T20:20:16Z

src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs

+            if (filterQuery.FilterOperation == FilterOperations.@in || filterQuery.FilterOperation == FilterOperations.nin)
+                return CallGenericWhereContainsMethod(source,filterQuery);
+            else
+                return CallGenericWhereMethod(source, filterQuery);


To keep convention same as for Sort and it's CallGenericOrderMethod, there is new CallGenericWhereContainsMethod and CallGenericWhereMethod for filtering. In the future this convention can be extended for fields Select method, for example CallGenericSelectMethod...

milosloub · 2018-09-30T20:24:12Z

src/JsonApiDotNetCore/Internal/Query/BaseAttrQuery.cs

+    /// It elimines boilerplate of providing specified type(AttrQuery or RelatedAttrQuery) 
+    /// while filter and sort operations and eliminates plenty of methods to keep DRY principles 
+    /// </summary>
+    public abstract class BaseAttrQuery


All shared properties (Attribute and Relationship) are located here. All implementations should use these default properties to keep DRY. Also this is the main class that can be cached for filter, sort and fields methods and make always one instance of Attribute and Relationship classes (in the future)

ah I see, generally I only use abstract when I have abstract members and would use a protected constructor instead, but i think this is perfectly fine

Yes, it depends on use case. I was inspired by EF approach and it's table inheritance - abstract base classes holds shared properties for all descendants and prevents creating new instances directly

https://www.learnentityframeworkcore.com/inheritance/table-per-hierarchy

milosloub · 2018-09-30T20:28:17Z

src/JsonApiDotNetCore/Services/QueryComposer.cs

@@ -30,8 +30,7 @@ public string Compose(IJsonApiContext jsonApiContext)
        private string ComposeSingleFilter(FilterQuery query)
        {
            var result = "&filter";
-            var operation = string.IsNullOrWhiteSpace(query.Operation) ? query.Operation : query.Operation + ":";
-            result += QueryConstants.OPEN_BRACKET + query.Attribute + QueryConstants.CLOSE_BRACKET + "=" + operation + query.Value;
+            result += QueryConstants.OPEN_BRACKET + query.Attribute + QueryConstants.CLOSE_BRACKET + "=" + query.OperationType + QueryConstants.COLON + query.Value;


If we want to use only enum typed operations, there can't be empty string

Just so I understand what you mean, you're not suggesting that the following is invalid:

?filter[prop]=val

Just that at this particular point in the code we don't need to check for null because we've already handled the default case—specifically in QueryParser.ParseFilterOperationAndValue?

Exactly, that was my thought

milosloub · 2018-09-30T20:34:50Z

src/JsonApiDotNetCore/Services/QueryParser.cs

                }
            }

            return queries;
        }

+        [Obsolete("This method is not used anymore! Use " + nameof(ParseFilterOperationAndValue) + " method with FilterOperations operation return value." +
+            "Operation as string is not used at all.")]


If we want to use FilterOperations base on enum, this method must be replaced by new ParseFilterOperationAndValue() which eliminates string operation value.
This is old cascade:
Parse() calls ParseFilterQuery() that calls ParseFilterOperation() which returns string op and string value
This is new cascade:
Parse() calls ParseFilterQuery() that calls ParseFilterOperationAndValue() which returns FilterOperations op and string value. This completely eliminates the ability to override the ParseFilterOperation(), and will be ignored.
Can be this problem?

I think it's fine. I'll need to work with some of our users to make sure this doesn't introduce non-trivial problems. But, let's clean up the language a bit 😄

[Obsolete("Use " + nameof(ParseFilterOperationAndValue) + " method instead.")]

On the same note, ParseFilterOperationAndValue should be protected virtual so the behavior can be overriden if necessary (like ParseFilterOperation).

Ok, if it is problem, I can revert all filter stuff. For now I changed access attribute to protected virtual and cleaned up language - sorry for my English representation, I'm from Czech republic (Europe) 😄

milosloub · 2018-09-30T20:37:03Z

src/JsonApiDotNetCore/Services/QueryParser.cs

+        /// </summary>
+        /// <param name="input"></param>
+        /// <returns></returns>
+        public static (FilterOperations operation, string value) ParseFilterOperationAndValue(string input)


Optimized parsing values and operations in one method. It's combination of ParseFilterOperation and GetFilterOperationOld methods.

milosloub · 2018-09-30T20:38:21Z

test/UnitTests/Services/QueryComposerTests.cs

@@ -56,7 +56,7 @@ public void Can_ComposeLessThan_FilterStringForUrl()
            // act
            var filterString = queryComposer.Compose(_jsonApiContext.Object);
            // assert
-            Assert.Equal("&filter[attribute]=le:value&filter[attribute2]=value2", filterString);
+            Assert.Equal("&filter[attribute]=le:value&filter[attribute2]=eq:value2", filterString);


here again - empty string operation are not allowed

We have to allow queries with default equality operations. This will be way too much work for some code bases.

Ok. What about QueryComposer without change and don't make Operation property (FilterQuery) obsolete?

milosloub · 2018-09-30T20:39:24Z

test/UnitTests/Services/QueryParser_Tests.cs

@@ -99,7 +100,7 @@ public void Filters_Properly_Parses_DateTime_Without_Operation()

            // assert
            Assert.Equal(dt, querySet.Filters.Single(f => f.Attribute == "key").Value);
-            Assert.Equal(string.Empty, querySet.Filters.Single(f => f.Attribute == "key").Operation);
+            Assert.Equal(FilterOperations.eq, querySet.Filters.Single(f => f.Attribute == "key").OperationType);


Same use case as in the comment above

Update to latest

milosloub · 2018-10-09T06:39:56Z

@jaredcnance Hi, I fixed branche conflicts and found code like this:

            {
                var defaultSortOrder = _resourceDefinition.DefaultSort();
                if (defaultSortOrder != null && defaultSortOrder.Count > 0)
                {
                    foreach (var sortProp in defaultSortOrder)
                    {
                        // this is dumb...add an overload, don't allocate for no reason
                        entities.Sort(new SortQuery(sortProp.Item2, sortProp.Item1));
                    }
                }
            }

I think this pull request can solve DefaultSort() by using SortQuery with new constructor overload. Please check before release stable 3.0 version.

jaredcnance

hey, sorry it took me so long to get to this. things are looking pretty good. mostly minor comments. i need to spend some more time on this mostly because this PR makes a lot of API changes (removals and additions). I want to make sure we get it as right as we can on the first pass to reduce future churn of the API.

jaredcnance · 2018-10-14T23:30:36Z

src/JsonApiDotNetCore/Internal/Query/AttrFilterQuery.cs

        public AttrFilterQuery(
            IJsonApiContext jsonApiContext,
            FilterQuery filterQuery)
+            : base(jsonApiContext, 
+                  null,
+                  filterQuery.Attribute, 


any reason we can't just pass filterQuery rather than expanding it?

Yes, first solution was different ( no class BaseAttrQuery). Now I can pass filterQuery and BaseAttrQuery will have BaseQuery in constructor parameters. Good point!

jaredcnance · 2018-10-14T23:37:24Z

src/JsonApiDotNetCore/Internal/Query/AttrFilterQuery.cs

-
-        private AttrAttribute GetAttribute(string attribute) =>
-            _jsonApiContext.RequestEntity.Attributes.FirstOrDefault(attr => attr.Is(attribute));
+        [Obsolete("Use " + nameof(Attribute) + " property of " + nameof(BaseAttrQuery) + "class. This property is shared for all AttrQuery and RelatedAttrQuery (filter,sort..) implementations.")]


Let's simplify the language here a bit, maybe:

[Obsolete("Use " + nameof(BaseAttrQuery.Attribute) + " insetad.")]

I'm not sure the rest adds any extra value and increases build output noise.

jaredcnance · 2018-10-14T23:40:50Z

src/JsonApiDotNetCore/Internal/Query/BaseAttrQuery.cs

+
+        public BaseAttrQuery(IJsonApiContext jsonApiContext, string relationship, string attribute)
+        {
+            _jsonApiContext = jsonApiContext;


Let's depend on the IContextGraph interface rather than IJsonApiContext..

Maybe I don't understand, but all the time AttrFilterQuery and RelatedAttrQuery depends on IJsonApiContext, so why IContextGraph?

yes, you're correct 😞 , the idea was to only depend on the interface that you actually need. #253 elaborates on this a bit more.

jaredcnance · 2018-10-14T23:41:54Z

src/JsonApiDotNetCore/Internal/Query/BaseFilterQuery.cs

+            FilterOperation = op;
+        }
+
+        [Obsolete("To resolve operation use enum typed " + nameof(FilterQuery.OperationType) + " property of "+ nameof(FilterQuery) +" class")]


Simplify:

[Obsolete("Use " + nameof(FilterQuery.OperationType) + " instead.")]

jaredcnance · 2018-10-14T23:42:56Z

src/JsonApiDotNetCore/Internal/Query/BaseQuery.cs

+
+namespace JsonApiDotNetCore.Internal.Query
+{
+    public abstract class BaseQuery


why abstract?

Because concept is same as BaseAttrQuery, that only holds info about attributes, relationship etc. I don't know about use case, when client should create new instance of this BaseQuery, only descendant class (FilterQuery, SortQuery). Or do you have some idea where to use it without abstract?

jaredcnance · 2018-10-14T23:53:13Z

src/JsonApiDotNetCore/Services/QueryComposer.cs

@@ -30,8 +30,7 @@ public string Compose(IJsonApiContext jsonApiContext)
        private string ComposeSingleFilter(FilterQuery query)
        {
            var result = "&filter";
-            var operation = string.IsNullOrWhiteSpace(query.Operation) ? query.Operation : query.Operation + ":";
-            result += QueryConstants.OPEN_BRACKET + query.Attribute + QueryConstants.CLOSE_BRACKET + "=" + operation + query.Value;
+            result += QueryConstants.OPEN_BRACKET + query.Attribute + QueryConstants.CLOSE_BRACKET + "=" + query.OperationType + QueryConstants.COLON + query.Value;


Just so I understand what you mean, you're not suggesting that the following is invalid:

?filter[prop]=val

Just that at this particular point in the code we don't need to check for null because we've already handled the default case—specifically in QueryParser.ParseFilterOperationAndValue?

jaredcnance · 2018-10-14T23:56:00Z

src/JsonApiDotNetCore/Services/QueryParser.cs

-
-        private string GetFilterOperation(string value)
+        [Obsolete("Delete also when " + nameof(ParseFilterOperation) + " deleted.")]
+        private string GetFilterOperationOld(string value)


By renaming this the Obsolete attribute is useless.

jaredcnance · 2018-10-14T23:59:09Z

src/JsonApiDotNetCore/Services/QueryParser.cs

                }
            }

            return queries;
        }

+        [Obsolete("This method is not used anymore! Use " + nameof(ParseFilterOperationAndValue) + " method with FilterOperations operation return value." +
+            "Operation as string is not used at all.")]


I think it's fine. I'll need to work with some of our users to make sure this doesn't introduce non-trivial problems. But, let's clean up the language a bit 😄

[Obsolete("Use " + nameof(ParseFilterOperationAndValue) + " method instead.")]

On the same note, ParseFilterOperationAndValue should be protected virtual so the behavior can be overriden if necessary (like ParseFilterOperation).

jaredcnance · 2018-10-14T23:59:28Z

src/JsonApiDotNetCore/Services/QueryParser.cs

+            if (values.Length == 1)
+                return (FilterOperations.eq, input);
+            // prefix:value
+            else if (values.Length == 2)


no need for else and as a matter of style, let's add newlines before commented lines. also, a few other comments about parsing here:

We're saying ?filter[prop]=some:value is not allowed and will automatically result in a 400. I don't think that's correct. I think we should try to parse the operation and if the value is not an operation key, we simply use equals and maybe log a debug message.

So, basically all of this gets simplified to:

if (values.Length == 1) return (FilterOperations.eq, input); var (operation, succeeded) = GetFilterOperation(values[0]); var valueStartIndex = (containsOperation) ? 1 : 0; var value = string.Join(QueryConstants.COLON_STR, values.Skip(valueStartIndex)); return (operation, value);

...or something along those lines.

?filter[prop]=some:value, is it really posible? What about GetFilterExpressionLambda in IQueryableExtensions...if I understand it correctly, if client provides custom filter operation, it will be stopped here.

jaredcnance · 2018-10-15T00:02:44Z

src/JsonApiDotNetCore/Services/QueryParser.cs

+        /// </summary>
+        /// <param name="operation">String represented operation</param>
+        /// <returns></returns>
+        public static (FilterOperations operation, bool succeeded) GetFilterOperation(string operation)


Should be private. But, is this method really necessary? Why not just Enum.TryParse(operation, out FilterOperations opertion)?

Fixed access modifier. I made method, because of two calls

jaredcnance · 2018-10-15T00:30:59Z

I think this pull request can solve DefaultSort() by using SortQuery with new constructor overload. Please check before release stable 3.0 version.

I'm not sure what you mean here. Can you elaborate? If you'd like a quicker mode of communication, feel free to ping me on Gitter.

milosloub · 2018-10-15T06:51:33Z

@jaredcnance Hi, I've got one suggestion - what about reverting all filter stuff and make focus only on nested sort feature? Most of comments refer to filtering, so maybe it would be better to split this PR to two separate PR and deliver nested sort as soon as posible. After while I'll contact you on gitter - good idea

milosloub · 2018-10-15T08:23:20Z

I think this pull request can solve DefaultSort() by using SortQuery with new constructor overload. Please check before release stable 3.0 version.

I'm not sure what you mean here. Can you elaborate? If you'd like a quicker mode of communication, feel free to ping me on Gitter.

I mean something like this:

        internal List<SortQuery> DefaultSort()
        {
            var defaultSortOrder = GetDefaultSortOrder();
            if (defaultSortOrder != null && defaultSortOrder.Count > 0)
            {
                var order = new List<SortQuery>();
                foreach (var sortProp in defaultSortOrder)
                {
                    // TODO: error handling, log or throw?
                    if (sortProp.Item1.Body is MemberExpression memberExpression)
                    {
                        var attribute = _contextEntity.Attributes.SingleOrDefault(a => a.InternalAttributeName != memberExpression.Member.Name);
                        if (attribute == null)
                            throw new JsonApiException(500, "Some exception");
                        order.Add(new SortQuery(sortProp.Item2, attribute.PublicAttributeName));
                    }
                }
                return order;
            }
            return null;
        }

And then usage in DefaultEntityRepository:

        public virtual IQueryable<TEntity> Sort(IQueryable<TEntity> entities, List<SortQuery> sortQueries)
        {
            if (sortQueries != null && sortQueries.Count > 0)
                return entities.Sort(_jsonApiContext, sortQueries);

            if (_resourceDefinition != null)
            {
                var defaultSortOrder = _resourceDefinition.DefaultSort();
                if (defaultSortOrder != null && defaultSortOrder.Count > 0)
                {
                    // Direct usage of defined sortqueries
                    return entities.Sort(_jsonApiContext, defaultSortOrder);
                }
            }

            return entities;
        }

Update to latest

milosloub · 2018-10-16T23:57:44Z

Changes description:

Person.cs -> only new Age property for testing purposes
DefaultEntityRepository.cs -> Sort() call with IJsonApiContext param - same approach as Filter()
IQueryableExtensions.cs -> most changes.
-New Sort methods with IJsonApiContext param, old ones as obsolete,
-One Filter() method instead of two - now require BaseFilterQuery param instead of AttrFilterQuery
and RelatedAttrFilterQuery.
-Add methods CallGenericWhereContainsMethod() and CallGenericWhereMethod() to keep
convention just like CallGenericOrderMethod()
AttrFilterQuery - GetAttribute() now in ancestor BaseAttrQuery, PropertyValue and FilterOperation moved to BaseFilterQuery (neccessary due to IQueryableExtension.Filter() method)
new AttrSortQuery - same approach as AttrFilterQuery
new BaseAttrQuery - shared ancestor, that handles getting Attribute, Relationship from IJsonApiContext
BaseFilterQuery - new ancestor BaseAttrQuery. PropertyValue and FilterOperation are here as shared properties for AttrFilterQuery and RelatedAttrFilterQuery
new BaseQuery.cs - ancestor of all Queries( FilterQuery, SortQuery...). Holds info about atribute, relationship and if attribute belongs to relationship
FilterQuery - new ancestor BaseQuery. Attribute and IsAttributeOfRelationship now in BaseQuery
QueryConstants - new DOT constant
RelatedAttrFilterQuery - same as point 4. + GetRelationship() now in ancestor BaseAttrQuery
new RelatedAttrSortQuery - same approach as RelatedAttrFilterQuery
SortQuery - new constructor overload - just like FilterQuery only require string attribute - this is the key for nested sort feature. Previous constructor as obsolete
QueryParser - building SortQuery by new constructor overload. BuildFilterQuery method deleted
TodoItemsControllerTests - Added two tests for nested sort feature

Spacing changes only in:

QueryParser_Tests.cs
QueryComposerTests.cs
SparseFieldSetTests.cs
QuerySet.cs

You can ignore these files

jaredcnance

Some minor comments/questions. After this round we'll go ahead and merge and I'll release as a beta. Thanks for all your work!

jaredcnance · 2018-10-17T01:13:05Z

src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs

        public static IOrderedQueryable<TSource> Sort<TSource>(this IQueryable<TSource> source, SortQuery sortQuery)
        {
+            // For clients using SortQuery constructor with string based parameter
+            if (sortQuery.SortedAttribute == null)
+                throw new JsonApiException(400, $"It's not possible to provide {nameof(SortQuery)} without {nameof(SortQuery.SortedAttribute)} parameter." +


3 things:

Does this need to be added? It looks like SortedAttribute is deprecated, but it looks like we're still setting the value. Has something else changed, or does this handle an error case that wasn't handled properly in the current version and would have resulted in a null ref exception below?

The language refers to internal method usage (i.e. it's directed at the application developer), but a 400 is returned. If this is a request error, the language should be targeted towards the client / client application: e.g. "Cannot sort on an empty attribute". OR if this is a developer concern it should return a 500.

Is this something that should be handled earlier in the request lifecycle?

Still reviewing so it's possible I'm missing something.

It's not necessary. I did it because of compatible solution for clients, that use SortQuery instance with SortedAttribute. As developer you can do this:
a) create SortQuery() based on SortedAttribute and then call IQueryable.Sort(List<SortQuery>) => it's OK
b) create SortQuery() based on SortedAttribute and then call IQueryable.Sort(IJsonApiContext, List<SortQuery>) => it's not ok, there is Exception
c) create SortQuery() based on string Attribute and then call IQueryable.Sort(IJsonApiContext, List<SortQuery>) => it's OK
d) create SortQuery() based on string Attribute and then call IQueryable.Sort(List<SortQuery>) =>it's not ok, there is Exception

I have question - do you use deprecations like "It's not use at all, fix it to new object"? If yes, it's better to me - then I can have 3 Sort methods instead of 6 and use it only with IJsonApiContext parameter. But this leads to one problem - developers, that calls IQueryable.Sort() method receive error during build time (because of missing obligatory IJsonApiContext param). What do you think about this?
2) If "deprecation question" in point 1. is valid, this makes no sense. Otherwise I change 400 to 500 - it's definitely developer concern.
3) This also depends on point 1.

Yes, we use deprecations based on the following strategy (where possible):

Introduce new APIs and deprecate the old, keeping functionality intact using an [Obsolete(..., error: false)]

Fully deprecate in a major release with [Obsolete(..., error: true)]

Remove API in next major release

This was originally documented here and is being finalized as part of #290.

So, ideally we would deprecate and leave the existing path fully intact so developers can upgrade with minimal impact to their builds. Then in the next major release we force a build error. However, I would rather force a new build error than a new runtime error.

In this case, the deprecation is on the Sort(List<SortQuery>) but the old functionality should still be fine, correct? If so, then the deprecation on the method itself should be sufficient. As far as I can tell this is not a 400 error. So, ultimately this comes down to a single question:

Was a new failure mode introduced for this method? i.e. was something added that would cause SortedAttribute to be null? If the answer is no, then the exception is fine just make it a 500 (but the root cause for this exception seems to be unrelated to this change). If there is a new failure mode, then I need to understand what it is.

Ok, that deprecation strategy makes sence. So now it's made as the first point [Obsolete(..., error: false)]
I would rather also jump to step two [Obsolete(..., error: true)], because of eliminating bad constructor usage of SortQuery combined with bad Sort() method.

Explanation:

Before PR - QueryParser creates SortQuery list based on SortedAttribute, then
DefaultEntityRepository calls Sort(List<SortQuery>)

After PR - QueryParser creates SortQuery list based on Attribute( as string), then DefaultEntityRepository calls Sort(IJsonApiContext, List<SortQuery>)

So everything is fine and old functionality is kept. But JsonApiException will be thrown only in this specific case: Update to new version -> developer creates new SortQuery instance somewhere in his code -> use inappropriate Sort() method. This means that for standard implementations everything is OK, but for custom implementations this "criss cross" usage may occur.

I see two solutions here:

Make [Obsolete(..., error: true)] - ideally on Sort() methods and SortedAttribute also, then JsonApiException is not neccessary at all

Keep it as it is and only change JsonApiException from 400 to 500.

What do you think about this?

Okay thanks for the explanation. Let's go with option 1. I'd prefer a build time error with a clear explanation over a new runtime exception.

jaredcnance · 2018-10-17T01:17:39Z

src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs

+            if (sortQueries == null || sortQueries.Count == 0)
+                return source;
+
+            var orderedEntities = source.Sort(jsonApiContext, sortQueries[0]);


why do we need this here rather than run it all in the for loop?

It's current implementation, from me there is no change, please check. Also I think this approach is fine

jaredcnance · 2018-10-17T01:37:46Z

src/JsonApiDotNetCore/Internal/Query/BaseAttrQuery.cs

+
+        public BaseAttrQuery(IJsonApiContext jsonApiContext, string relationship, string attribute)
+        {
+            _jsonApiContext = jsonApiContext;


yes, you're correct 😞 , the idea was to only depend on the interface that you actually need. #253 elaborates on this a bit more.

jaredcnance · 2018-10-17T01:38:56Z

src/JsonApiDotNetCore/Internal/Query/BaseAttrQuery.cs

+    /// It elimines boilerplate of providing specified type(AttrQuery or RelatedAttrQuery) 
+    /// while filter and sort operations and eliminates plenty of methods to keep DRY principles 
+    /// </summary>
+    public abstract class BaseAttrQuery


ah I see, generally I only use abstract when I have abstract members and would use a protected constructor instead, but i think this is perfectly fine

wisepotato · 2018-10-19T07:13:55Z

Oh man this looks good!

Milos and others added 9 commits September 24, 2018 20:34

Nested sort - Init

94fdff2

Requested changes

8d46f52

AttrQuery and RelatedAttrQuery constructor overload.

2f3e5d1

Dramatical reduction of repeating code blocks.

b04ee1f

Fixed tests - following item can has same age value as previsous(< wa…

d63ff38

…s replaced by <=and vice versa)

Sort breaking change fixed. I have to pay more attention to public me…

487bef9

…thods

Back to the roots. There was few breaking changes, so I have to turn …

18014f6

…back to provide compatibility.

Nested sort - feature with refactor

61dacea

Fix sort and remove abstract BaseFilterQuery

188de7d

milosloub commented Sep 30, 2018

View reviewed changes

milosloub added 4 commits October 9, 2018 08:15

Fix: filter divided by comma

43071ed

Merge pull request #3 from json-api-dotnet/master

7e9fc76

Update to latest

Latest

4102288

Fix: branche conficts in DefaultEntityRepository

2e304cc

jaredcnance reviewed Oct 15, 2018

View reviewed changes

Milos added 2 commits October 15, 2018 10:25

Requested changes

d97affd

Fix conflicts

47d4fcd

Milos and others added 6 commits October 15, 2018 11:07

Fix?

cc3564d

Strange conflict fix

5b6e614

Merge pull request #5 from json-api-dotnet/master

95b3acc

Update to latest

Update to latest

5e1db73

Fix: ContextGraph => ResourceGraph

04172a4

Revert filter back

0be3300

jaredcnance reviewed Oct 17, 2018

View reviewed changes

finish deprecating old APIs

d1bc2cb

jaredcnance merged commit 2221d57 into json-api-dotnet:master Oct 18, 2018

milosloub deleted the Nested-sort branch October 19, 2018 08:13

Nested sort #416

Nested sort #416

Conversation

milosloub commented Sep 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milosloub Sep 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milosloub Sep 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milosloub Sep 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milosloub Oct 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milosloub commented Oct 9, 2018 • edited Loading

jaredcnance left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaredcnance Oct 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milosloub Oct 15, 2018 • edited Loading

Choose a reason for hiding this comment

jaredcnance commented Oct 15, 2018

milosloub commented Oct 15, 2018

milosloub commented Oct 15, 2018

milosloub commented Oct 16, 2018 • edited Loading

jaredcnance left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milosloub Oct 17, 2018 • edited Loading

Choose a reason for hiding this comment

jaredcnance Oct 17, 2018 • edited Loading

Choose a reason for hiding this comment

milosloub Oct 17, 2018 • edited Loading

Choose a reason for hiding this comment

jaredcnance Oct 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wisepotato commented Oct 19, 2018

milosloub commented Sep 30, 2018 •

edited

Loading

milosloub Sep 30, 2018 •

edited

Loading

milosloub Sep 30, 2018 •

edited

Loading

milosloub Sep 30, 2018 •

edited

Loading

milosloub Oct 15, 2018 •

edited

Loading

milosloub commented Oct 9, 2018 •

edited

Loading

jaredcnance left a comment •

edited

Loading

jaredcnance Oct 14, 2018 •

edited

Loading

milosloub Oct 15, 2018 •

edited

Loading

milosloub commented Oct 16, 2018 •

edited

Loading

milosloub Oct 17, 2018 •

edited

Loading

jaredcnance Oct 17, 2018 •

edited

Loading

milosloub Oct 17, 2018 •

edited

Loading

jaredcnance Oct 17, 2018 •

edited

Loading