-
Notifications
You must be signed in to change notification settings - Fork 644
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Propagate correlation id from gallery to search service #2892
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
||
using System; | ||
using System.Net.Http; | ||
using Correlator.Extensions; | ||
|
||
namespace NuGet.Services.Search.Client.Correlation | ||
{ | ||
public class CorrelationIdProvider | ||
{ | ||
public Guid CorrelationId { get; private set; } | ||
|
||
public CorrelationIdProvider() | ||
{ | ||
CorrelationId = Guid.NewGuid(); | ||
} | ||
|
||
public CorrelationIdProvider(HttpRequestMessage request) | ||
{ | ||
CorrelationId = request.GetClientCorrelationId(); | ||
|
||
if (CorrelationId == Guid.Empty) | ||
{ | ||
CorrelationId = Guid.NewGuid(); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
||
namespace NuGet.Services.Search.Client.Correlation | ||
{ | ||
/// <summary> | ||
/// Indicated that the implementing class supports correlation id propogation, and thus excepts CorrelationIdProvider. | ||
/// </summary> | ||
public interface ICorrelated | ||
{ | ||
CorrelationIdProvider CorrelationIdProvider { get; set; } | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,11 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<packages> | ||
<package id="Microsoft.AspNet.WebApi.Client" version="5.2.3" targetFramework="net452" /> | ||
<package id="Microsoft.AspNet.WebApi.Core" version="5.1.2" targetFramework="net452" /> | ||
<package id="Microsoft.Bcl" version="1.1.10" targetFramework="net452" /> | ||
<package id="Microsoft.Bcl.Build" version="1.0.21" targetFramework="net452" /> | ||
<package id="Microsoft.Net.Http" version="2.2.29" targetFramework="net452" /> | ||
<package id="Newtonsoft.Json" version="6.0.8" targetFramework="net452" /> | ||
<package id="NuGet.Services.Platform.Client" version="3.0.29-r-master" targetFramework="net452" /> | ||
<package id="WebApiCorrelator" version="1.1.0.0" targetFramework="net452" /> | ||
</packages> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,21 +9,22 @@ | |
using System.Web.Http; | ||
using System.Web.Http.OData; | ||
using System.Web.Http.OData.Query; | ||
using NuGet.Frameworks; | ||
using NuGet.Services.Search.Client.Correlation; | ||
using NuGet.Versioning; | ||
using NuGetGallery.Configuration; | ||
using NuGetGallery.Infrastructure; | ||
using NuGetGallery.Infrastructure.Lucene; | ||
using NuGetGallery.OData; | ||
using NuGetGallery.OData.QueryInterceptors; | ||
using NuGetGallery.WebApi; | ||
using QueryInterceptor; | ||
using WebApi.OutputCache.V2; | ||
using NuGet.Versioning; | ||
using NuGet.Frameworks; | ||
using NuGetGallery.Infrastructure.Lucene; | ||
|
||
// ReSharper disable once CheckNamespace | ||
namespace NuGetGallery.Controllers | ||
{ | ||
public class ODataV2FeedController | ||
public class ODataV2FeedController | ||
: NuGetODataController | ||
{ | ||
private const int MaxPageSize = SearchAdaptor.MaxPageSize; | ||
|
@@ -33,7 +34,7 @@ public class ODataV2FeedController | |
private readonly ISearchService _searchService; | ||
|
||
public ODataV2FeedController( | ||
IEntityRepository<Package> packagesRepository, | ||
IEntityRepository<Package> packagesRepository, | ||
ConfigurationService configurationService, | ||
ISearchService searchService) | ||
: base(configurationService) | ||
|
@@ -62,6 +63,8 @@ public async Task<IHttpActionResult> Get(ODataQueryOptions<V2FeedPackage> option | |
HijackableQueryParameters hijackableQueryParameters = null; | ||
if (SearchHijacker.IsHijackable(options, out hijackableQueryParameters) && _searchService is ExternalSearchService) | ||
{ | ||
SetCorrelation(); | ||
|
||
packages = await SearchAdaptor.FindByIdAndVersionCore( | ||
_searchService, GetTraditionalHttpContext().Request, packages, | ||
hijackableQueryParameters.Id, hijackableQueryParameters.Version, curatedFeed: null); | ||
|
@@ -131,6 +134,8 @@ private async Task<IHttpActionResult> GetCore(ODataQueryOptions<V2FeedPackage> o | |
// try the search service | ||
try | ||
{ | ||
SetCorrelation(); | ||
|
||
packages = await SearchAdaptor.FindByIdAndVersionCore( | ||
_searchService, GetTraditionalHttpContext().Request, packages, id, version, curatedFeed: null); | ||
|
||
|
@@ -169,8 +174,8 @@ public IHttpActionResult GetPropertyFromPackages(string propertyName, string id, | |
{ | ||
switch (propertyName.ToLowerInvariant()) | ||
{ | ||
case "id": return Ok(id); | ||
case "version": return Ok(version); | ||
case "id": return Ok(id); | ||
case "version": return Ok(version); | ||
} | ||
|
||
return BadRequest("Querying property " + propertyName + " is not supported."); | ||
|
@@ -181,8 +186,8 @@ public IHttpActionResult GetPropertyFromPackages(string propertyName, string id, | |
[HttpPost] | ||
[CacheOutput(ServerTimeSpan = NuGetODataConfig.SearchCacheTimeInSeconds, ClientTimeSpan = NuGetODataConfig.SearchCacheTimeInSeconds)] | ||
public async Task<IHttpActionResult> Search( | ||
ODataQueryOptions<V2FeedPackage> options, | ||
[FromODataUri]string searchTerm = "", | ||
ODataQueryOptions<V2FeedPackage> options, | ||
[FromODataUri]string searchTerm = "", | ||
[FromODataUri]string targetFramework = "", | ||
[FromODataUri]bool includePrerelease = false) | ||
{ | ||
|
@@ -202,7 +207,7 @@ public async Task<IHttpActionResult> Search( | |
targetFramework = targetFrameworkList[0]; | ||
} | ||
} | ||
|
||
// Peform actual search | ||
var packages = _packagesRepository.GetAll() | ||
.Include(p => p.PackageRegistration) | ||
|
@@ -212,6 +217,8 @@ public async Task<IHttpActionResult> Search( | |
.AsNoTracking(); | ||
|
||
// todo: search hijack should take options instead of manually parsing query options | ||
SetCorrelation(); | ||
|
||
var query = await SearchAdaptor.SearchCore( | ||
_searchService, GetTraditionalHttpContext().Request, packages, searchTerm, targetFramework, includePrerelease, curatedFeed: null); | ||
|
||
|
@@ -223,14 +230,14 @@ public async Task<IHttpActionResult> Search( | |
var pagedQueryable = query | ||
.Take(options.Top != null ? Math.Min(options.Top.Value, MaxPageSize) : MaxPageSize) | ||
.ToV2FeedPackageQuery(GetSiteRoot(), _configurationService.Features.FriendlyLicenses); | ||
|
||
return QueryResult(options, pagedQueryable, MaxPageSize, totalHits, (o, s, resultCount) => | ||
{ | ||
// The nuget.exe 2.x list command does not like the next link at the bottom when a $top is passed. | ||
// Strip it of for backward compatibility. | ||
if (o.Top == null || (resultCount.HasValue && o.Top.Value >= resultCount.Value)) | ||
{ | ||
return SearchAdaptor.GetNextLink(Request.RequestUri, resultCount, new {searchTerm, targetFramework, includePrerelease}, o, s); | ||
return SearchAdaptor.GetNextLink(Request.RequestUri, resultCount, new { searchTerm, targetFramework, includePrerelease }, o, s); | ||
} | ||
return null; | ||
}); | ||
|
@@ -245,8 +252,8 @@ public async Task<IHttpActionResult> Search( | |
[HttpGet] | ||
[CacheOutput(ServerTimeSpan = NuGetODataConfig.SearchCacheTimeInSeconds, ClientTimeSpan = NuGetODataConfig.SearchCacheTimeInSeconds)] | ||
public async Task<IHttpActionResult> SearchCount( | ||
ODataQueryOptions<V2FeedPackage> options, | ||
[FromODataUri]string searchTerm = "", | ||
ODataQueryOptions<V2FeedPackage> options, | ||
[FromODataUri]string searchTerm = "", | ||
[FromODataUri]string targetFramework = "", | ||
[FromODataUri]bool includePrerelease = false) | ||
{ | ||
|
@@ -261,9 +268,9 @@ public IHttpActionResult GetUpdates( | |
ODataQueryOptions<V2FeedPackage> options, | ||
[FromODataUri]string packageIds, | ||
[FromODataUri]string versions, | ||
[FromODataUri]bool includePrerelease, | ||
[FromODataUri]bool includeAllVersions, | ||
[FromODataUri]string targetFrameworks = "", | ||
[FromODataUri]bool includePrerelease, | ||
[FromODataUri]bool includeAllVersions, | ||
[FromODataUri]string targetFrameworks = "", | ||
[FromODataUri]string versionConstraints = "") | ||
{ | ||
if (string.IsNullOrEmpty(packageIds) || string.IsNullOrEmpty(versions)) | ||
|
@@ -377,5 +384,13 @@ where versionLookup[p.PackageRegistration.Id] | |
} | ||
return updates; | ||
} | ||
|
||
private void SetCorrelation() | ||
{ | ||
if (_searchService is ICorrelated) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we do this in the constructor instead? That way we don't have to think of calling this method explicitly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't... the Request is null in the ctor, so I have no correlation id to pass along There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right! Overlooked that one :S |
||
{ | ||
((ICorrelated)_searchService).CorrelationIdProvider = new CorrelationIdProvider(Request); | ||
} | ||
} | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whitespace seems gone?