Skip to content

Conversation

@davidfowl
Copy link
Member

  • Today when we make endpoints for the ModelDataSource, we apply the callbacks to the EndpointBuilder before creating the endpoint. This happens every time anyone accesses the endpoints property. Instead of re-applying the conventions, we do it once on Build. We also throw if more conventions are added after building.
  • Added tests

Fixes #34199

- Today when we make endpoints for the ModelDataSource, we apply the callbacks to the EndpointBuilder before creating the endpoint. This happens every time anyone accesses the endpoints property. Instead of re-applying the conventions, we do it once on Build. We also throw if more conventions are added after building.
- Added tests
@davidfowl davidfowl requested a review from javiercn as a code owner September 9, 2021 17:24
@ghost ghost added the area-runtime label Sep 9, 2021
@davidfowl davidfowl added old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels feature-routing and removed area-runtime labels Sep 9, 2021
@davidfowl davidfowl requested a review from pranavkm September 9, 2021 17:25
{
foreach (var convention in _conventions)
// Only apply the conventions once
var conventions = Interlocked.Exchange(ref _conventions, null);
Copy link
Contributor

Choose a reason for hiding this comment

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

It's interlocked because we're worried about Add / Build being called concurrently?

Copy link
Member Author

@davidfowl davidfowl Sep 9, 2021

Choose a reason for hiding this comment

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

I'm just being extra defensive. It's unclear if these APIs are intended to be called thread safe. This is just to prevent any corruption.

// Access endpoints a couple of times to make sure it gets built
_ = dataSource.Endpoints;
_ = dataSource.Endpoints;
_ = dataSource.Endpoints;
Copy link
Contributor

Choose a reason for hiding this comment

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

3 is more than a "couple".

@davidfowl
Copy link
Member Author

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2021

@davidfowl davidfowl enabled auto-merge (squash) September 9, 2021 18:13
@davidfowl davidfowl merged commit 8576c4b into main Sep 9, 2021
@davidfowl davidfowl deleted the davidfowl/duplicate-metadata branch September 9, 2021 19:14
@ghost ghost added this to the 7.0-preview1 milestone Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature-routing old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RouteEndpoint metadata from convention builders are duplicated

3 participants