Simplify DocumentUri implementation

[CyrusNajmabadi]

No description provided.

[CyrusNajmabadi]

note Uri.Equals doesn't exist. it calls through to object.Equals

[dibarbet]

I am pretty sure Uri.Equals exists and I am pretty sure we need to use it in some cases to handle casing - https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.Uri/src/System/Uri.cs#L1745

[dibarbet]

I am pretty sure Uri.Equals exists and I am pretty sure we need to use it in some cases to handle casing - https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.Uri/src/System/Uri.cs#L1745

[CyrusNajmabadi]

I am pretty sure Uri.Equals exists and I am pretty sure we need to use it in some cases to handle casing - dotnet/runtime@main/src/libraries/System.Private.Uri/src/System/Uri.cs#L1745

That's instance Equals. But this is the static Equals(a, b) being called. Taht has a very simple impl which just does some null checking, and then calls into instance .Equals.

So the prior code was correct, as is the new code. It's just that saying Uri.Equals(a, b) is pointless. It's a call to Object.Equals(a, b).

[dibarbet]

That's instance Equals. But this is the static Equals(a, b) being called

ah of course, for some reason I read Uri as a property.

[dibarbet]

I don't think this is always correct. Nothing is constraining the System.Uri to be an absolute uri,, it could be relative and AbsoluteUri will throw

I think that also even round tripping an absolute URI like this could change how it is parsed - for example possibly percent encoding the Uri twice.

[CyrusNajmabadi]

i def agree. though that was the existing logic:

    public DocumentUri(Uri parsedUri)
    {
        UriString = parsedUri.AbsoluteUri;
        _parsedUriLazy = new(() => parsedUri);
    }

Any thoughts on what we should do?

[dibarbet]

edit - the first part of my statement was wrong. it looks like its already doing AbsoluteUri below. But I believe the second still applies. I'm not sure there is a guarantee that Uri.AbsoluteUri == new Uri(Uri.AbsoluteUr)).AbsoluteUri

[dibarbet]

Is it possible to keep the DocumentUri(Uri parsedUri) ctor, and assign the Uri to the Optional<Uri> like we did before?

For the absolute part, it must be the callers that guarantee an absolute URI. I would be an ok with an explicit throw if not absolute here.

[CyrusNajmabadi]

ooh. very good catch. i actually had that, but accidentally deleted it. it def should remain!

stale