Take DocumentContextFactory off of RenameEndpoint

[ryanbrandenburg]

We need DocumentContextFactory access to happen in a controlled manner which is not vulnerable to concurrency issues, which almost certainly means it needs to happen on the RazorRequestContextFactory.

One idea is to have something like IAllDocumentContextsHandler (terrible name) which when defined as part of the handler instructs the RequestContextFactory to get the DocumentContext's for ALL document in the solution (probably using code similar to https://github.com/dotnet/razor/blob/main/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Refactoring/RenameEndpoint.cs#L150).

[davidwengier]

There are actually two issues here.

1. What my comment was mainly about, is that DocumentContextFactory only works for open documents, so using it in rename, for example, could mean we simply fail a rename request if we have to do something with a closed document. I don't know if thats true for this specific example, because Roslyn helps us out here by issuing edits for closed generated documents, which we get as didChange requests, and remap, but the theory is there regardless. I think #6960 is probably the best solution here, since it will at least make it a deliberate choice to restrict to open documents. Maybe i'll see if I can put my head down and do that in a few weeks when I'm in a bad timezone, though working on planes has never been my forte.
2. What this issue is tracking, about potential document sync issues because during the course of a message being handled, an endpoint can just get the "latest" version of a document. Rather than a new type of handler for this, I think we should just move to Roslyn's immutable snapshot model, where each handler is passed the entire solution it should operate on. Whether its actually Roslyn's types or not, I don't mind. This is probably exactly what you meant by IAllDocumentContextsHandler, but I wasn't sure :)

[ryanbrandenburg]

where each handler is passed the entire solution it should operate on.

That's kind of what I was getting at but I wouldn't say that EVERY Handler should get the entire solution because that's kind of a big hammer. But then again maybe if the solution is already calculated (I think it is?) it wouldn't actually matter.

In your comment you gave the example of URIPresentation, but I actually think they're doing it wrong too because they're just using to DocumentResolver (which is what the DocumentContextFactory does under the covers IIRC).

[davidwengier]

I wouldn't say that EVERY Handler should get the entire solution because that's kind of a big hammer.

In Roslyn there is a mechanism for opting out (RequiresLSPSolution), and the only handlers that do are didOpen, didChange and didClose. I don't think Razor would be any different, because even an endpoint that did nothing but delegate would still need to presumably map document positions to/from the delegated server. Either way, having it be optional is not a big deal.

But then again maybe if the solution is already calculated (I think it is?) it wouldn't actually matter.

Yeah, one of the reasons for explicitly declaring if a handler mutates the solution, means that the solution state can be re-used among any number of requests until there is a mutating one. There is a bunch of code in Roslyn that caches things for this purpose.

In your comment you gave the example of URIPresentation, but I actually think they're doing it wrong too because they're just using to DocumentResolver

That is the solution to issue 1, but yeah, it doesn't help with issue 2. So its half-right :)