Refactor int? where 'null = no limit' and '> 0 = some'

commonsensesoftware · commonsensesoftware · commit 61d158442657 · 2022-10-03T14:19:15.000-07:00
diff --git a/src/Common/src/Common.OData.ApiExplorer/Conventions/DefaultODataQueryOptionDescriptionProvider.cs b/src/Common/src/Common.OData.ApiExplorer/Conventions/DefaultODataQueryOptionDescriptionProvider.cs
@@ -214,15 +214,15 @@ protected virtual string DescribeTop( ODataQueryOptionDescriptionContext context
             throw new ArgumentNullException( nameof( context ) );
         }
 
-        if ( !context.MaxTop.HasValue || context.MaxTop.Value <= 0 )
+        if ( context.MaxTop.NoLimitOrNone() )
         {
             return ODataExpSR.TopQueryOptionDesc;
         }
 
         return GetOrCreateBuilder()
                 .Append( ODataExpSR.TopQueryOptionDesc )
                 .Append( Space )
-                .AppendFormat( CurrentCulture, ODataExpSR.MaxValueDesc, context.MaxTop.Value )
+                .AppendFormat( CurrentCulture, ODataExpSR.MaxValueDesc, context.MaxTop )
                 .ToString();
     }
 
@@ -238,15 +238,15 @@ protected virtual string DescribeSkip( ODataQueryOptionDescriptionContext contex
             throw new ArgumentNullException( nameof( context ) );
         }
 
-        if ( !context.MaxSkip.HasValue || context.MaxSkip.Value <= 0 )
+        if ( context.MaxSkip.NoLimitOrNone() )
         {
             return ODataExpSR.SkipQueryOptionDesc;
         }
 
         return GetOrCreateBuilder()
                 .Append( ODataExpSR.SkipQueryOptionDesc )
                 .Append( Space )
-                .AppendFormat( CurrentCulture, ODataExpSR.MaxValueDesc, context.MaxSkip.Value )
+                .AppendFormat( CurrentCulture, ODataExpSR.MaxValueDesc, context.MaxSkip )
                 .ToString();
     }
 
diff --git a/src/Common/src/Common.OData.ApiExplorer/Conventions/ODataAttributeVisitor.cs b/src/Common/src/Common.OData.ApiExplorer/Conventions/ODataAttributeVisitor.cs
@@ -90,51 +90,52 @@ private void VisitEnableQuery( IReadOnlyList<EnableQueryAttribute> attributes )
             AllowedQueryOptions = attribute.AllowedQueryOptions;
 
             if ( attribute.MaxAnyAllExpressionDepth != @default.MaxAnyAllExpressionDepth )
-            {
-            if ( context.MaxAnyAllExpressionDepth == @default.MaxAnyAllExpressionDepth )
             {
                 context.MaxAnyAllExpressionDepth = attribute.MaxAnyAllExpressionDepth;
             }
 
-            if ( context.MaxExpansionDepth == @default.MaxExpansionDepth )
+            if ( attribute.MaxExpansionDepth != @default.MaxExpansionDepth )
             {
                 context.MaxExpansionDepth = attribute.MaxExpansionDepth;
             }
 
-            if ( context.MaxNodeCount == @default.MaxNodeCount )
+            if ( attribute.MaxNodeCount != @default.MaxNodeCount )
             {
                 context.MaxNodeCount = attribute.MaxNodeCount;
             }
 
-            if ( context.MaxOrderByNodeCount == @default.MaxOrderByNodeCount )
+            if ( attribute.MaxOrderByNodeCount != @default.MaxOrderByNodeCount )
             {
                 context.MaxOrderByNodeCount = attribute.MaxOrderByNodeCount;
             }
 
-            if ( context.MaxSkip != @default.MaxSkip )
+            if ( attribute.MaxSkip != @default.MaxSkip )
             {
                 context.MaxSkip = attribute.MaxSkip;
             }
 
-            if ( context.MaxTop != @default.MaxTop )
+            if ( attribute.MaxTop != @default.MaxTop )
             {
                 context.MaxTop = attribute.MaxTop;
             }
 
-            if ( !string.IsNullOrEmpty( attribute.AllowedOrderByProperties ) )
+            if ( string.IsNullOrEmpty( attribute.AllowedOrderByProperties ) )
             {
-                var properties = attribute.AllowedOrderByProperties.Split( new[] { ',' }, RemoveEmptyEntries );
-                var allowedOrderByProperties = context.AllowedOrderByProperties;
-                var comparer = StringComparer.OrdinalIgnoreCase;
+                continue;
+            }
 
-                for ( var j = 0; j < properties.Length; j++ )
-                {
-                    var property = properties[j].Trim();
+            var properties = attribute.AllowedOrderByProperties.Split( new[] { ',' }, RemoveEmptyEntries );
+            var allowedOrderByProperties = context.AllowedOrderByProperties;
+            var comparer = StringComparer.OrdinalIgnoreCase;
+
+            for ( var j = 0; j < properties.Length; j++ )
+            {
+                var property = properties[j].Trim();
 
-                    if ( !string.IsNullOrEmpty( property ) && !allowedOrderByProperties.Contains( property, comparer ) )
-                    {
-                        allowedOrderByProperties.Add( property );
-                    }
+                if ( !string.IsNullOrEmpty( property ) &&
+                     !allowedOrderByProperties.Contains( property, comparer ) )
+                {
+                    allowedOrderByProperties.Add( property );
                 }
             }
         }
@@ -218,10 +219,16 @@ private void VisitCount( ModelBoundQuerySettings querySettings )
 
     private void VisitMaxTop( ModelBoundQuerySettings querySettings )
     {
-        if ( querySettings.MaxTop != null && querySettings.MaxTop.Value > 0 )
+        if ( querySettings.MaxTop.Unset() )
         {
-            context.MaxTop = querySettings.MaxTop;
+            return;
         }
+
+        context.MaxTop = querySettings.MaxTop;
+
+        // calling the Page() configuration sets MaxTop and PageSize,
+        // which is implied to enable $top and $skip
+        AllowedQueryOptions |= Skip | Top;
     }
 
     private void Visit<TSetting>(
diff --git a/src/Common/src/Common.OData.ApiExplorer/Conventions/ODataValidationSettingsConvention.cs b/src/Common/src/Common.OData.ApiExplorer/Conventions/ODataValidationSettingsConvention.cs
@@ -174,7 +174,7 @@ private AllowedQueryOptions GetQueryOptions( DefaultQuerySettings settings, ODat
             queryOptions |= Select;
         }
 
-        if ( settings.MaxTop != null && settings.MaxTop.Value > 0 )
+        if ( settings.MaxTop.NoLimitOrSome() )
         {
             context.MaxTop = settings.MaxTop;
         }
diff --git a/src/Common/src/Common.OData.ApiExplorer/Conventions/ODataValidationSettingsExtensions.cs b/src/Common/src/Common.OData.ApiExplorer/Conventions/ODataValidationSettingsExtensions.cs
@@ -21,12 +21,12 @@ internal static void CopyFrom( this ODataValidationSettings original, ODataValid
         original.MaxNodeCount = source.MaxNodeCount;
         original.MaxOrderByNodeCount = source.MaxOrderByNodeCount;
 
-        if ( source.MaxSkip != null )
+        if ( source.MaxSkip.NoLimitOrNone() )
         {
             original.MaxSkip = source.MaxSkip;
         }
 
-        if ( source.MaxTop != null )
+        if ( source.MaxTop.NoLimitOrSome() )
         {
             original.MaxTop = source.MaxTop;
         }
diff --git a/src/Common/src/Common.OData.ApiExplorer/NullableExtensions.cs b/src/Common/src/Common.OData.ApiExplorer/NullableExtensions.cs
@@ -0,0 +1,17 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
+
+namespace System;
+
+using System.Runtime.CompilerServices;
+
+internal static class NullableExtensions
+{
+    [MethodImpl( MethodImplOptions.AggressiveInlining )]
+    public static bool Unset( this int? value ) => value.HasValue && value.Value == 0;
+
+    [MethodImpl( MethodImplOptions.AggressiveInlining )]
+    public static bool NoLimitOrSome( this int? value ) => !value.HasValue || value.Value > 0;
+
+    [MethodImpl( MethodImplOptions.AggressiveInlining )]
+    public static bool NoLimitOrNone( this int? value ) => !value.HasValue || value.Value <= 0;
+}

Original file line number	Diff line number	Diff line change
`@@ -214,15 +214,15 @@ protected virtual string DescribeTop( ODataQueryOptionDescriptionContext context`
`214`	`214`	`throw new ArgumentNullException( nameof( context ) );`
`215`	`215`	`}`
`216`	`216`
`217`		`- if ( !context.MaxTop.HasValue \|\| context.MaxTop.Value <= 0 )`
	`217`	`+ if ( context.MaxTop.NoLimitOrNone() )`
`218`	`218`	`{`
`219`	`219`	`return ODataExpSR.TopQueryOptionDesc;`
`220`	`220`	`}`
`221`	`221`
`222`	`222`	`return GetOrCreateBuilder()`
`223`	`223`	`.Append( ODataExpSR.TopQueryOptionDesc )`
`224`	`224`	`.Append( Space )`
`225`		`- .AppendFormat( CurrentCulture, ODataExpSR.MaxValueDesc, context.MaxTop.Value )`
	`225`	`+ .AppendFormat( CurrentCulture, ODataExpSR.MaxValueDesc, context.MaxTop )`
`226`	`226`	`.ToString();`
`227`	`227`	`}`
`228`	`228`
`@@ -238,15 +238,15 @@ protected virtual string DescribeSkip( ODataQueryOptionDescriptionContext contex`
`238`	`238`	`throw new ArgumentNullException( nameof( context ) );`
`239`	`239`	`}`
`240`	`240`
`241`		`- if ( !context.MaxSkip.HasValue \|\| context.MaxSkip.Value <= 0 )`
	`241`	`+ if ( context.MaxSkip.NoLimitOrNone() )`
`242`	`242`	`{`
`243`	`243`	`return ODataExpSR.SkipQueryOptionDesc;`
`244`	`244`	`}`
`245`	`245`
`246`	`246`	`return GetOrCreateBuilder()`
`247`	`247`	`.Append( ODataExpSR.SkipQueryOptionDesc )`
`248`	`248`	`.Append( Space )`
`249`		`- .AppendFormat( CurrentCulture, ODataExpSR.MaxValueDesc, context.MaxSkip.Value )`
	`249`	`+ .AppendFormat( CurrentCulture, ODataExpSR.MaxValueDesc, context.MaxSkip )`
`250`	`250`	`.ToString();`
`251`	`251`	`}`
`252`	`252`
Original file line number	Diff line number	Diff line change
`@@ -90,51 +90,52 @@ private void VisitEnableQuery( IReadOnlyList<EnableQueryAttribute> attributes )`
`90`	`90`	`AllowedQueryOptions = attribute.AllowedQueryOptions;`
`91`	`91`
`92`	`92`	`if ( attribute.MaxAnyAllExpressionDepth != @default.MaxAnyAllExpressionDepth )`
`93`		`- {`
`94`		`- if ( context.MaxAnyAllExpressionDepth == @default.MaxAnyAllExpressionDepth )`
`95`	`93`	`{`
`96`	`94`	`context.MaxAnyAllExpressionDepth = attribute.MaxAnyAllExpressionDepth;`
`97`	`95`	`}`
`98`	`96`
`99`		`- if ( context.MaxExpansionDepth == @default.MaxExpansionDepth )`
	`97`	`+ if ( attribute.MaxExpansionDepth != @default.MaxExpansionDepth )`
`100`	`98`	`{`
`101`	`99`	`context.MaxExpansionDepth = attribute.MaxExpansionDepth;`
`102`	`100`	`}`
`103`	`101`
`104`		`- if ( context.MaxNodeCount == @default.MaxNodeCount )`
	`102`	`+ if ( attribute.MaxNodeCount != @default.MaxNodeCount )`
`105`	`103`	`{`
`106`	`104`	`context.MaxNodeCount = attribute.MaxNodeCount;`
`107`	`105`	`}`
`108`	`106`
`109`		`- if ( context.MaxOrderByNodeCount == @default.MaxOrderByNodeCount )`
	`107`	`+ if ( attribute.MaxOrderByNodeCount != @default.MaxOrderByNodeCount )`
`110`	`108`	`{`
`111`	`109`	`context.MaxOrderByNodeCount = attribute.MaxOrderByNodeCount;`
`112`	`110`	`}`
`113`	`111`
`114`		`- if ( context.MaxSkip != @default.MaxSkip )`
	`112`	`+ if ( attribute.MaxSkip != @default.MaxSkip )`
`115`	`113`	`{`
`116`	`114`	`context.MaxSkip = attribute.MaxSkip;`
`117`	`115`	`}`
`118`	`116`
`119`		`- if ( context.MaxTop != @default.MaxTop )`
	`117`	`+ if ( attribute.MaxTop != @default.MaxTop )`
`120`	`118`	`{`
`121`	`119`	`context.MaxTop = attribute.MaxTop;`
`122`	`120`	`}`
`123`	`121`
`124`		`- if ( !string.IsNullOrEmpty( attribute.AllowedOrderByProperties ) )`
	`122`	`+ if ( string.IsNullOrEmpty( attribute.AllowedOrderByProperties ) )`
`125`	`123`	`{`
`126`		`- var properties = attribute.AllowedOrderByProperties.Split( new[] { ',' }, RemoveEmptyEntries );`
`127`		`- var allowedOrderByProperties = context.AllowedOrderByProperties;`
`128`		`- var comparer = StringComparer.OrdinalIgnoreCase;`
	`124`	`+ continue;`
	`125`	`+ }`
`129`	`126`
`130`		`- for ( var j = 0; j < properties.Length; j++ )`
`131`		`- {`
`132`		`- var property = properties[j].Trim();`
	`127`	`+ var properties = attribute.AllowedOrderByProperties.Split( new[] { ',' }, RemoveEmptyEntries );`
	`128`	`+ var allowedOrderByProperties = context.AllowedOrderByProperties;`
	`129`	`+ var comparer = StringComparer.OrdinalIgnoreCase;`
	`130`	`+`
	`131`	`+ for ( var j = 0; j < properties.Length; j++ )`
	`132`	`+ {`
	`133`	`+ var property = properties[j].Trim();`
`133`	`134`
`134`		`- if ( !string.IsNullOrEmpty( property ) && !allowedOrderByProperties.Contains( property, comparer ) )`
`135`		`- {`
`136`		`- allowedOrderByProperties.Add( property );`
`137`		`- }`
	`135`	`+ if ( !string.IsNullOrEmpty( property ) &&`
	`136`	`+ !allowedOrderByProperties.Contains( property, comparer ) )`
	`137`	`+ {`
	`138`	`+ allowedOrderByProperties.Add( property );`
`138`	`139`	`}`
`139`	`140`	`}`
`140`	`141`	`}`
`@@ -218,10 +219,16 @@ private void VisitCount( ModelBoundQuerySettings querySettings )`
`218`	`219`
`219`	`220`	`private void VisitMaxTop( ModelBoundQuerySettings querySettings )`
`220`	`221`	`{`
`221`		`- if ( querySettings.MaxTop != null && querySettings.MaxTop.Value > 0 )`
	`222`	`+ if ( querySettings.MaxTop.Unset() )`
`222`	`223`	`{`
`223`		`- context.MaxTop = querySettings.MaxTop;`
	`224`	`+ return;`
`224`	`225`	`}`
	`226`	`+`
	`227`	`+ context.MaxTop = querySettings.MaxTop;`
	`228`	`+`
	`229`	`+ // calling the Page() configuration sets MaxTop and PageSize,`
	`230`	`+ // which is implied to enable $top and $skip`
	`231`	`+ AllowedQueryOptions \|= Skip \| Top;`
`225`	`232`	`}`
`226`	`233`
`227`	`234`	`private void Visit<TSetting>(`
Original file line number	Diff line number	Diff line change
`@@ -174,7 +174,7 @@ private AllowedQueryOptions GetQueryOptions( DefaultQuerySettings settings, ODat`
`174`	`174`	`queryOptions \|= Select;`
`175`	`175`	`}`
`176`	`176`
`177`		`- if ( settings.MaxTop != null && settings.MaxTop.Value > 0 )`
	`177`	`+ if ( settings.MaxTop.NoLimitOrSome() )`
`178`	`178`	`{`
`179`	`179`	`context.MaxTop = settings.MaxTop;`
`180`	`180`	`}`
Original file line number	Diff line number	Diff line change
`@@ -21,12 +21,12 @@ internal static void CopyFrom( this ODataValidationSettings original, ODataValid`
`21`	`21`	`original.MaxNodeCount = source.MaxNodeCount;`
`22`	`22`	`original.MaxOrderByNodeCount = source.MaxOrderByNodeCount;`
`23`	`23`
`24`		`- if ( source.MaxSkip != null )`
	`24`	`+ if ( source.MaxSkip.NoLimitOrNone() )`
`25`	`25`	`{`
`26`	`26`	`original.MaxSkip = source.MaxSkip;`
`27`	`27`	`}`
`28`	`28`
`29`		`- if ( source.MaxTop != null )`
	`29`	`+ if ( source.MaxTop.NoLimitOrSome() )`
`30`	`30`	`{`
`31`	`31`	`original.MaxTop = source.MaxTop;`
`32`	`32`	`}`