Fix concurrent request handling for OpenAPI documents

[xC0dex]

Fix: Concurrent request for OpenApi documents

• You've read the Contributor Guide and Code of Conduct.
• You've included unit or integration tests for your change, where applicable.
• You've included inline docs for your change, where applicable.
• There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Description

This PR allows concurrent requests for OpenApi documents by switching from a Dictionary to a ConcurrentDictionary. The fields _schemas, SchemasByReference and _operationTransformerContextCache had to be changed to ConcurrentDictionary to pass the test. Additionally, I updated _referenceIdCounter for consistency.

Fixes #57876

[xC0dex]

@dotnet-policy-service agree

[martincostello]

I wonder if using Parallel with a bigger number of operations would make this more likely to always run requests that race against each other?

[xC0dex]

Thanks. That was an excellent idea. I switched to Parallel.ForAsync and it turned out that the _operationTransformerContextCache field must also be a ConcurrentDictionary.

[captainsafia]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[captainsafia]

LGTM! 🚀

[captainsafia]

@BrennanConroy shared some feedback here that we can use a different overload of the GetOrAdd method here to avoid another possible race condition:

Suggested change

	return _schemas.GetOrAdd(key, valueFactory(key));
	return _schemas.GetOrAdd(key, valueFactory, key);

With this pattern, the GetOrAdd method will do a check for the schema key in the dictionary before calling into the factory.

The current overload used will always call the valueFactory even if the key already exists in the dictionary.

In this case, the valueFactory were calling is an invocation into the JsonSchemaMapper. I dunno if it is particularly prone to race issues but it still doesn't hurt to avoid unnecessarily calling into it.

[xC0dex]

Thank you for the feedback. One small PR and I already learned a lot!
Unfortunately, the suggested overload does not work because it expects a valueFactory of type Func<TKey, TArg, TValue>. Luckily, there is another overload, so I could do the following:

- return _schemas.GetOrAdd(key, valueFactory(key));
+ return _schemas.GetOrAdd(key, _ => valueFactory(key));

[martincostello]

I think this should work, and will avoid needing to allocate a closure:

return _schemas.GetOrAdd(key, valueFactory);