One drive escape function for key as segment staring with colon by KanishManuja-MS · Pull Request #1621 · OData/odata.net

KanishManuja-MS · 2020-01-08T19:42:22Z

Issues

This pull request fixes the escape function uri.

Description

The previous implementation of escape function uri was not able to distinguish if a request intended to use an escape function or a key-value began with a ':' when the key is expressed as a segment. Therefore, the parsing would fail for the requests with key as segment with value starting with ':' with an error that no escape function was found for the type.

Now, we change how we parse segments and give precedence to an escape function if one exists for the type otherwise treat it as a key-value.

This pull requests

Additional work necessary

Release notes need to mention these changes when merged.

mikepizzo · 2020-01-13T23:46:36Z

                parenthesisExpression = null;
            }
+            else if (segmentText[segmentText.Length - 1] == ':')
+            {


add a comment describing logic -- is this to support syntax such as:
entityset(key):/escape-function-path? #Closed

mikepizzo · 2020-01-13T23:47:06Z

+            else if (segmentText[segmentText.Length - 1] == ':')
+            {
+
+                if (segmentText.Length < 2 ||


remove empty blank line #Closed

mikepizzo · 2020-01-13T23:55:14Z


+
+
            return false;


remove extra blank lines
#Closed

mikepizzo · 2020-01-14T00:04:57Z

+
+            bool trybindingEscapeFunction = false;
+
+            if (segmentText.Length > 0 && segmentText[segmentText.Length - 1] == ':' && previous.EdmType != null)


segmentText.Length > 0 && [](start = 16, length = 25)

is this ever called with a zero-length segmentText, or should this just be an assert? #Closed

mikepizzo · 2020-01-14T04:00:34Z

+            IEdmType bindingType = null;
+            if (previous != null)
+            {
+                bindingType = (previous is EachSegment) ? previous.TargetEdmType : previous.EdmType;


(previous is EachSegment) [](start = 30, length = 25)

Do we have a test to validate this? In general, I don't think we should support an escape function (or any other function) following $each. The only thing that should validly follow $each is a bound action. #Closed

I have removed the check of $each at this point for escape functions if it should not be supported.

In reply to: 366143523 [](ancestors = 366143523)

mikepizzo · 2020-01-14T04:02:14Z

            Assert.Equal("xyz/abc", ((ConstantNode)parameter.Value).Value);
        }

-        [Fact]


Is this test no longer valid? #Closed

mikepizzo · 2020-01-14T04:03:32Z

-            var odataException = Assert.Throws(test);
-            Assert.Equal(ODataErrorStrings.RequestUriProcessor_NoBoundEscapeFunctionSupported("NS.OneDrive"), odataException.Message);
-        }
-


What is the behavior for this url? It should still throw an invalid url, right? because root: is not a valid identifier? #Closed

Correct. Added the test back with the updated error message.

In reply to: 366144030 [](ancestors = 366144030)

mikepizzo · 2020-01-14T04:05:28Z

-        [InlineData("EntitySet('key'):/xyz:/:/perm", new[] { "EntitySet('key')", ":xyz:", ":perm" })]
-        [InlineData(":/xyz::/perm", new[] { ":xyz:", ":perm" })]
-        [InlineData(":/xyz:/:/perm", new[] { ":xyz:", ":perm" })]
-        [InlineData(":/xyz://///:/perm", new[] { ":xyz:", ":perm" })]


Why are all variations of this test removed? Aren't most of these still valid? Any that are now invalid should have added negative tests. #Closed

+1, we can change the expected to make the test valid again.
Because all the patterns are valid case, we can use this test cases to see the ParsePath works expected for these patterns.

In reply to: 366144313 [](ancestors = 366144313)

mikepizzo · 2020-01-14T04:06:04Z

-        [Theory]
-        [InlineData("root::/abc", "root::")]
-        [InlineData("EntitySet('key')::/abc", "EntitySet('key')::")]
-        public void ParseInvalidEscapeUriPathShouldThrow(string pattern, string segment)


Are these now both valid? What is the behavior? #Closed

Yes, both are valid for this step. These will throw an Unrecogonized Path Exception in the E2E scenario while binding.

In reply to: 366144414 [](ancestors = 366144414)

mikepizzo · 2020-01-14T04:08:16Z

remove trailing space (and trailing blank line) #Closed

Refers to: src/Microsoft.OData.Core/UriParser/Parsers/UriPathParser.cs:117 in 267863a. [](commit_id = 267863a, deletion_comment = False)

mikepizzo · 2020-01-14T04:33:55Z

+
+            bool trybindingEscapeFunction = false;
+
+            if (segmentText.Length > 0 && segmentText[segmentText.Length - 1] == ':' && previous.EdmType != null)


segmentText.Length > 0 [](start = 16, length = 22)

Should this be an assert? Didn't we validate that segmentText.Length > 0 in ExtractSegmentIdentifierAndParenthesisExpression on line 118? Do we not call ExtractSegmentIdentifierAndParenthesisExpression before calling TryHanbdleAsKeySegment? #Closed

This is the remains from a previous iteration. In my previous logic, I had permitted empty segment text. This is not needed now.

In reply to: 366148775 [](ancestors = 366148775)

mikepizzo · 2020-01-14T04:43:51Z

+                    // If nothing to bind for key then bind the escape function directly. 
+                    if (segmentText.Length == 0)
+                    {
+                        return this.TryBindEscapeFunction();


You have already done a somewhat expensive operation to figure out what the escape function is. You shouldn't have to redo that work when you call TryBindEscapeFunction. Can you split TryBindEscapeFunction into two pieces, the first the determines the function and the second that takes that information and does the actual binding, and call the second function from here and line 403? #Closed

Instead of splitting it into two functions I decided to have this take in an optional function.

In reply to: 366150654 [](ancestors = 366150654)

See my comments in iteration 22. I still think we can do a little refactoring to make the code cleaner and more performant.

In reply to: 366662603 [](ancestors = 366662603,366150654)

mikepizzo · 2020-01-14T04:45:57Z

+                    // If nothing to bind for key then bind the escape function directly. 
+                    if (segmentText.Length == 0)
+                    {
+                        return this.TryBindEscapeFunction();


return [](start = 24, length = 6)

Logic is a bit confusing, since you aren't handling it as a key, but you are handling the segment. Maybe add to the comment "and return true so that the caller knows the segment has been handled."

Also, don't you know at this point that TryBindEscapeFunction will return true (since you already verified the escape function? If, for some reason, TryBindEscapeFunction returned false, would you want this method to return false or would you want the logic below to run? (and determine if the single ":" character was to be treated as a key).
#Closed

It can throw an error even if there is an escape function bound to this type. It will never return false as of now. More details in comments, let me know how you would like me to proceed.
I changed the code as I thought was the best possible way to handle it.
PS: I dislike the int passed to tell how many segments to remove from the list.

In reply to: 366150964 [](ancestors = 366150964)

mikepizzo · 2020-01-14T05:06:53Z

+
+            if (bindingType == null)  // escape function is only for bind function
+
+            {


remove blank line #Closed

mikepizzo · 2020-01-14T05:09:22Z

-            return function.FullName();
+            parenthesisExpression = function.Parameters.ElementAt(1).Name + "='" + (isComposableRequired ? identifier.Substring(0, identifier.Length - 1) : identifier.Substring(0)) + "'";
+            qualifiedName = function.FullName();
+


remove space at end of line. #Closed

mikepizzo · 2020-01-14T05:10:01Z

+                return false;
+                // should be an exception instead. 
+                //throw ExceptionUtil.CreateBadRequestError("Bound function is not composable.");
            }


is this work to be done? #Closed

mikepizzo · 2020-01-14T05:12:15Z

+            {
+                identifier = identifier.Substring(0, identifier.Length - 1);
+                tryBindingEscapeFunction = true;
+            }


same logic appears multiple times -- perhaps encapsulate in a common function? #Closed

mikepizzo · 2020-01-14T05:18:51Z

                this.TryBindKeyFromParentheses(parenthesisExpression);
+
+                if (tryBindingEscapeFunction && !this.TryBindEscapeFunction())
+                {


So the logic is, for each segment that could be followed by an escape function, if the identifier ends in ':', try binding the identifier up to the colon as that segment type, and see if the you can bind a function to it. If not, revert binding the segment and continue. Is that Right?

However, if the identifier ends in colon, and the segment successfully binds, then if there is no bound escape function then the only possibility is that the segment may be a key. Right? Does that simplify the subsequent logic? #Closed

Yes, you understanding is correct regarding what I am trying to do here. However, the segment may successfully bind as type which may not have escape function bound but the segment can then be bound to a type which may have an escape function. Therefore, it is incorrect to say that it will only bind to the key segment but yes, it is very likely that it will bind to the key segment.

Even if I take that assumption, the code will be simplified but not to a great extent.

In reply to: 366156392 [](ancestors = 366156392)

mikepizzo · 2020-01-14T20:26:21Z

        // Type cast

Need to add code (and test) for a type cast segment followed by an escape. i.e., Orders/NS.Order:/xyz/abc: and /Customers(2)/NS.Customer:/xyz/abc: #Closed

Refers to: src/Microsoft.OData.Core/UriParser/Parsers/ODataPathParser.cs:1251 in 267863a. [](commit_id = 267863a, deletion_comment = False)

mikepizzo · 2020-01-14T20:43:19Z


            this.TryBindKeySegmentIfNoResolvedParametersAndParenthesisValueExists(parenthesisExpression, returnType, resolvedParameters, segment);

+            if (tryBindingEscapeFunction && !this.TryBindEscapeFunction())


!this.TryBindEscapeFunction() [](start = 44, length = 29)

shouldn't this return false if bindescapefunction returns false, so the segment can be evaluated as a key? i.e., "/Customers(2)/NS.FindOrderComposable(orderName='name'):/xyz/abc:" currently succeeds, ignoring everything after Customers(2), and returns a path with 2 segments (Customers entity set and key) if there is no escape function defined that takes an order. #Resolved

mikepizzo · 2020-01-14T21:16:15Z

    private bool TryCreateSegmentForOperationImport(string identifier, string parenthesisExpression)

This needs to handle a function import followed by an escape function. #Closed

Refers to: src/Microsoft.OData.Core/UriParser/Parsers/ODataPathParser.cs:1018 in 267863a. [](commit_id = 267863a, deletion_comment = False)

xuzhg · 2020-01-14T21:16:36Z

revert #Resolved

xuzhg · 2020-01-14T21:19:23Z

            EdmModel model = new EdmModel();

            EdmEntityType orderType = new EdmEntityType("NS", "Order");
-            orderType.AddKeys(orderType.AddStructuralProperty("Id", EdmPrimitiveTypeKind.Int32));


Int32 [](start = 89, length = 5)

why do we have to change it? #Resolved

mikepizzo

🕐

Sreejithpin · 2020-01-15T18:53:46Z

+                    throw ExceptionUtil.CreateSyntaxError();
+                }
+
+


Remove Blank Space #Resolved

xuzhg · 2020-01-22T20:56:05Z

+        private bool TryCreateEscapeFunctionSegment(string segmentText)
+        {
+            int numberOfSegmentsParsed = this.parsedSegments.Count;
+            string newSegmentText = segmentText.Substring(0, segmentText.Length - 1);


string newSegmentText = segmentText.Substring(0, segmentText.Length - 1); [](start = 12, length = 73)

this line is abrupt. Maybe add a comment or a Debug.Assert(...) to make sure the input segment text has a ":" at the end? #Closed

xuzhg · 2020-01-22T20:57:04Z


+        private bool TryCreateEscapeFunctionSegment(string segmentText)
+        {
+            int numberOfSegmentsParsed = this.parsedSegments.Count;


int numberOfSegmentsParsed = this.parsedSegments.Count; [](start = 12, length = 55)

line 1274 can move to Line 1290, right? #Closed

Nope. BindSegmentForEscapeFunction changes the list of segments and it's count.

In reply to: 369798577 [](ancestors = 369798577)

mikepizzo · 2020-01-22T21:06:34Z


-            parenthesisExpression = function.Parameters.ElementAt(1).Name + "='" + (isComposableRequired ? identifier.Substring(1, identifier.Length - 2) : identifier.Substring(1)) + "'";
-            return function.FullName();
+            parenthesisExpression = function.Parameters.ElementAt(1).Name + "='" + (isComposableRequired ? identifier.Substring(0, identifier.Length - 1) : identifier.Substring(0)) + "'";


identifier.Substring(0) [](start = 156, length = 23)

isn't this just "identifier"? #Closed

xuzhg · 2020-01-22T21:07:38Z


+        /// Binds a  for an escape function segment.
+        /// The text of the segment.
+        private bool BindSegmentForEscapeFunction(string segmentText)


BindSegmentForEscapeFunction [](start = 21, length = 28)

maybe name it as "BindEscapeFunctionSegment", follow up the others #Resolved

xuzhg · 2020-01-22T21:09:42Z

+                if (this.TryCreateSegmentForOperationImport(identifier, parenthesisExpression))
+                {
+                    return true;
+                }


here looks a lot of codes duplicated , should be just call "CreateFirstSegment"... and in else to call "CreateNexteSegment"? #Resolved

That is what the previous iteration was doing. It added a lot of checks to existing cases. I did not want to degrade existing performance in the hot path.

In reply to: 369804646 [](ancestors = 369804646)

xuzhg · 2020-01-22T21:10:33Z


            this.parsedSegments.Add(segment);
-
+            


revert #Resolved

xuzhg

xuzhg · 2020-01-22T21:18:00Z

I have concerns about the duplicated codes in BindSegmentForEscapeFunction. It will double the cost to maintain. Others look good to me. So i approved it. Thanks. #ByDesign

mikepizzo · 2020-01-22T21:19:45Z

        /// The name of the segment
        /// The query portion
+        /// Type to which the operation is bound. It is an optional parameter that can be evalulated from the previous segment if it is null.
        /// Whether or not the identifier referred to an action.


In the case of the escape function, you actually know more than the binding type -- you know the actual specific function overload that you want to bind, parameters to pass, etc. Rather than passing in (possibly null) values to this method, I was suggesting that you factor this method into two. The first part of this method has the current signature (without the added bindingType), and it determines the binding type and specific function (i.e., everything down to 1143). And then this method calls a new method (also called by TryBindEscapeFunction) that takes the previous segment, resolved function, and resolved parameters and does the bind. #Closed

mikepizzo · 2020-01-22T21:34:33Z

+            string newIdentifier, newParenthesisExpression;
+            bool anotherEscapeFunctionStarts = false;
+
+            if (this.TryResolveEscapeFunction(bindingType, configuration.Model, out newIdentifier, out newParenthesisExpression, out anotherEscapeFunctionStarts))


this.configuration.Model, to be clear where configuration is coming from? Note that you don't really have to pass this, since TryResolveEscapeFunction is an instance method and has access to the model through the same configuration property. #Resolved

mikepizzo · 2020-01-22T21:40:02Z


-        private static string ResolveEscapeFunction(string identifier, IEdmType bindingType, IEdmModel model, out string parenthesisExpression)
+        private bool TryResolveEscapeFunction(IEdmType bindingType, IEdmModel model, out string qualifiedName, out string parenthesisExpression, out bool anotherEscapeFunctionStarts)
        {


Note that this function was originally written to be called within TryCreateSegmentForOperation, so its inputs/outputs may be a little different now that you are calling this before trying to bind the operation (you now have the actual function you need to bind). So think about what input/output parameters make sense in this new usage, and whether it should be called from TryBindEscapeFunction or the logic should be merged with TryBindEscapeFunction. #Closed

mikepizzo · 2020-01-22T21:41:08Z


+
+            ODataPathSegment previous = this.parsedSegments[this.parsedSegments.Count - 1];
            // $value


Why move this? #Closed

You mean why move the TryCreateEscapeFunctionSegment? I did that to not ExtractIdentifierAndParenthesis proactively for escape function segment. If it is successful, we may not need to at this point.

In reply to: 369818816 [](ancestors = 369818816)

See iteration 22: you had moved ODataPathSegment previous = this.parsedSegments[this.parsedSegments.Count - 1]; from line 1202 (immediately before using it to check previous.TargetKind == RequestTargetKind.Primitive, where is is first used) to line 1195 (before TryCreateValueSegment). It looks like it moved back in iteration 23.

In reply to: 369827716 [](ancestors = 369827716,369818816)

mikepizzo · 2020-01-22T21:46:00Z

        }

+        private bool TryCreateEscapeFunctionSegment(string segmentText)
+        {


assert that segmentText ends in ":"? #Closed

mikepizzo · 2020-01-22T21:57:04Z

+                anotherEscapeFunctionStarts = true;
+            }
+
+            bool isComposableRequired = identifier.Length >= 1 && identifier[identifier.Length - 1] == ':';


identifier.Length [](start = 40, length = 17)

add check for identifier != null. #Resolved

mikepizzo · 2020-01-22T22:04:27Z

+
+            bool isComposableRequired = identifier.Length >= 1 && identifier[identifier.Length - 1] == ':';
+
+            if ((function.Parameters.FirstOrDefault().Type.Definition != bindingType) || (isComposableRequired && !function.IsComposable) || (!isComposableRequired && function.IsComposable))


add comment describing logic. #Resolved

mikepizzo · 2020-01-22T22:08:18Z

if function == null (for example, because there were escape functions that matched the binding parameter, but they didn't meet the composability requirements), shouldn't we return false, rather than throw here?

Refers to: src/Microsoft.OData.Core/UriParser/Parsers/ODataPathParser.cs:1858 in da28656. [](commit_id = da28656, deletion_comment = False)

mikepizzo · 2020-01-22T22:09:20Z

@@ -1621,8 +1862,10 @@ private static string ResolveEscapeFunction(string identifier, IEdmType bindingT
                throw ExceptionUtil.CreateBadRequestError(ODataErrorStrings.RequestUriProcessor_EscapeFunctionMustHaveOneStringParameter(function.FullName()));
            }



isn't this already validated in IsUrlEscapeFunction? If so, maybe just assert here? #Closed

Nope, IsUrlEscapeFunction just returns the functions based on the annotation. Although it is unlikely that this will ever get thrown if people annotate their escape functions carefully.

In reply to: 369831446 [](ancestors = 369831446)

mikepizzo · 2020-01-22T22:23:32Z

+            if (function == null)
+            {
+                function = model.FindBoundOperations(bindingType).OfType().FirstOrDefault(f => f.IsComposable == isComposableRequired && IsUrlEscapeFunction(model, f));
+            }


Yikes! This is the third time we've called a fairly expensive FindBoundOperations and run a fairly expensive (and similar) LINQ query over the results. Any way we can optimize this? Maybe first determine the list of candidates:
List candidates = model.FindBoundOperations(bindingType).OfType().Where(f=>IsUrlEscapeFunction(model, f)).ToList();
and then pick the best from that (presumably rather small) list?
-exact match of bindingType and composability
-any (base) type and composability
-return false? (I think we decided composability had to exactly match?) #Closed

slightly different but did what we talked about.

In reply to: 369837925 [](ancestors = 369837925)

KanishManuja-MS · 2020-01-22T23:27:48Z

I agree. Can't think of a better solution though.

In reply to: 577390017 [](ancestors = 577390017)

mikepizzo · 2020-01-24T05:15:22Z

+
+                return true;
+
+            }


nit: remove blank line #Closed

mikepizzo · 2020-01-24T05:18:52Z

+        private static IEdmFunction FindBestMatchForEscapeFunction(IEnumerable candidtates, bool isComposable, IEdmType bindingType)
+        {
+            IEdmFunction bestCandidate = null;
+            foreach (IEdmFunction f in candidtates)


candidtates [](start = 39, length = 11)

nit: spelling #Resolved

mikepizzo · 2020-01-24T05:24:35Z

+                    continue;
+                }
+
+                if (f.Parameters != null && f.Parameters.Count() == 2 && f.Parameters.ElementAt(1).Type.IsString())


Parameters.Count() [](start = 46, length = 18)

nit: rather than repeatedly re-enumerating, probably more efficient to do ToList; then you can index for the first and second parameters. #Resolved

mikepizzo · 2020-01-24T05:33:18Z

+                    }
+                    else
+                    {
+                        bestCandidate = f;


You are resetting the bestCandidate each time, which means that, if there is not an exact match, you'll pick the last one that matches (which, I think, is likely to be the base type, rather than the most derived type). You could instead check to see if the current candidate has the current bestCandidate (if any) as a base type (recursively) and, if so, make the current candidate your new bestCandidate. #Closed

mikepizzo

KanishManuja-MS requested a review from xuzhg January 8, 2020 19:42

xuzhg reviewed Jan 8, 2020

View reviewed changes

Comment thread src/Microsoft.OData.Core/UriParser/Parsers/UriPathParser.cs Outdated

mikepizzo reviewed Jan 13, 2020

View reviewed changes

mikepizzo reviewed Jan 14, 2020

View reviewed changes

xuzhg reviewed Jan 14, 2020

View reviewed changes

Comment thread tools/CustomMSBuild/Versioning.props

xuzhg Jan 14, 2020 •

edited by KanishManuja-MS

Loading

Copy link
Copy Markdown

Contributor

Choose a reason for hiding this comment

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

revert #Resolved

xuzhg reviewed Jan 14, 2020

View reviewed changes

mikepizzo suggested changes Jan 14, 2020

View reviewed changes

KanishManuja-MS requested review from mikepizzo and xuzhg January 15, 2020 04:39

Sreejithpin reviewed Jan 15, 2020

View reviewed changes

Comment thread src/Microsoft.OData.Core/UriParser/Parsers/ODataPathParser.cs Outdated

Sreejithpin reviewed Jan 15, 2020

View reviewed changes

xuzhg reviewed Jan 22, 2020

View reviewed changes

mikepizzo reviewed Jan 22, 2020

View reviewed changes

xuzhg reviewed Jan 22, 2020

View reviewed changes

xuzhg approved these changes Jan 22, 2020

View reviewed changes

mikepizzo reviewed Jan 22, 2020

View reviewed changes

KanishManuja-MS requested a review from mikepizzo January 23, 2020 18:55

mikepizzo reviewed Jan 24, 2020

View reviewed changes

KanishManuja-MS requested a review from mikepizzo January 24, 2020 10:41

xuzhg added the Ready for review Use this label if a pull request is ready to be reviewed label Jan 24, 2020

mikepizzo approved these changes Jan 29, 2020

View reviewed changes

fix one drive escape function ambiguity with key as segment

bcfec25

KanishManuja-MS force-pushed the OneDriveEscapeFunction branch from 279f0e7 to bcfec25 Compare January 29, 2020 18:20

KanishManuja-MS merged commit a6f09b6 into OData:master Jan 29, 2020


		bool trybindingEscapeFunction = false;

		if (segmentText.Length > 0 && segmentText[segmentText.Length - 1] == ':' && previous.EdmType != null)


		if (bindingType == null) // escape function is only for bind function

		{


		this.TryBindKeySegmentIfNoResolvedParametersAndParenthesisValueExists(parenthesisExpression, returnType, resolvedParameters, segment);

		if (tryBindingEscapeFunction && !this.TryBindEscapeFunction())



		ODataPathSegment previous = this.parsedSegments[this.parsedSegments.Count - 1];
		// $value


		bool isComposableRequired = identifier.Length >= 1 && identifier[identifier.Length - 1] == ':';

		if ((function.Parameters.FirstOrDefault().Type.Definition != bindingType) \|\| (isComposableRequired && !function.IsComposable) \|\| (!isComposableRequired && function.IsComposable))

		@@ -1621,8 +1862,10 @@ private static string ResolveEscapeFunction(string identifier, IEdmType bindingT
		throw ExceptionUtil.CreateBadRequestError(ODataErrorStrings.RequestUriProcessor_EscapeFunctionMustHaveOneStringParameter(function.FullName()));
		}

Conversation

KanishManuja-MS commented Jan 8, 2020

Issues

Description

Additional work necessary

Uh oh!

Uh oh!

mikepizzo Jan 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jan 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jan 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jan 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jan 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jan 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jan 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KanishManuja-MS Jan 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jan 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jan 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo commented Jan 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikepizzo Jan 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jan 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jan 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jan 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jan 13, 2020 •

edited

Loading

mikepizzo Jan 13, 2020 •

edited

Loading

mikepizzo Jan 13, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited

Loading

KanishManuja-MS Jan 14, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited

Loading

mikepizzo commented Jan 14, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited

Loading

mikepizzo commented Jan 14, 2020 •

edited

Loading

mikepizzo Jan 14, 2020 •

edited by KanishManuja-MS

Loading

mikepizzo commented Jan 14, 2020 •

edited

Loading

xuzhg Jan 14, 2020 •

edited by KanishManuja-MS

Loading

xuzhg Jan 14, 2020 •

edited by KanishManuja-MS

Loading

Sreejithpin Jan 15, 2020 •

edited by KanishManuja-MS

Loading

xuzhg Jan 22, 2020 •

edited by mikepizzo

Loading

xuzhg Jan 22, 2020 •

edited by mikepizzo

Loading

mikepizzo Jan 22, 2020 •

edited

Loading