Skip to content

Commit e719a41

Browse files
authored
Add OData error code for deprecate response messages (#8297)
Address #8296
1 parent bfd25d5 commit e719a41

File tree

6 files changed

+53
-22
lines changed

6 files changed

+53
-22
lines changed

src/NuGetGallery/Controllers/ODataV1FeedController.cs

+6-3
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@
44
using System;
55
using System.Data.Entity;
66
using System.Linq;
7+
using System.Net;
78
using System.Net.Http;
89
using System.Threading.Tasks;
910
using System.Web.Http;
1011
using System.Web.Http.OData;
12+
using System.Web.Http.OData.Extensions;
1113
using System.Web.Http.OData.Query;
14+
using Microsoft.Data.OData;
1215
using NuGet.Services.Entities;
1316
using NuGetGallery.Configuration;
1417
using NuGetGallery.OData;
@@ -69,7 +72,7 @@ private IHttpActionResult Get(ODataQueryOptions<V1FeedPackage> options, bool isN
6972
{
7073
if (!isNonHijackEnabled)
7174
{
72-
return BadRequest(Strings.ODataDisabled);
75+
return DeprecatedRequest(Strings.ODataDisabled);
7376
}
7477

7578
if (!ODataQueryVerifier.AreODataOptionsAllowed(options, ODataQueryVerifier.V1Packages,
@@ -235,7 +238,7 @@ private async Task<IHttpActionResult> GetCoreAsync(
235238
// non-hijacked queries are disabled, stop here.
236239
if (!isNonHijackEnabled)
237240
{
238-
return BadRequest(Strings.ODataParametersDisabled);
241+
return DeprecatedRequest(Strings.ODataParametersDisabled);
239242
}
240243

241244
if (return404NotFoundWhenNoResults && !packages.Any())
@@ -372,7 +375,7 @@ private async Task<IHttpActionResult> SearchAsync(
372375

373376
if (!isNonHijackEnabled)
374377
{
375-
return BadRequest(Strings.ODataParametersDisabled);
378+
return DeprecatedRequest(Strings.ODataParametersDisabled);
376379
}
377380

378381
if (!ODataQueryVerifier.AreODataOptionsAllowed(options, ODataQueryVerifier.V1Search,

src/NuGetGallery/Controllers/ODataV2FeedController.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ private async Task<IHttpActionResult> GetAsync(
167167

168168
if (!isNonHijackEnabled)
169169
{
170-
return BadRequest(Strings.ODataParametersDisabled);
170+
return DeprecatedRequest(Strings.ODataParametersDisabled);
171171
}
172172

173173
// Reject only when try to reach database.
@@ -389,7 +389,7 @@ private async Task<IHttpActionResult> GetCoreAsync(
389389
{
390390
if (!isSimpleLookup)
391391
{
392-
return BadRequest(Strings.ODataParametersDisabled);
392+
return DeprecatedRequest(Strings.ODataParametersDisabled);
393393
}
394394

395395
customQuery = true;
@@ -557,7 +557,7 @@ private async Task<IHttpActionResult> SearchAsync(
557557

558558
if (!isNonHijackEnabled)
559559
{
560-
return BadRequest(Strings.ODataParametersDisabled);
560+
return DeprecatedRequest(Strings.ODataParametersDisabled);
561561
}
562562

563563
//Reject only when try to reach database.

src/NuGetGallery/OData/NuGetODataController.cs

+14
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@
33

44
using System;
55
using System.Linq;
6+
using System.Net;
67
using System.Web;
78
using System.Web.Http;
89
using System.Web.Http.OData;
10+
using System.Web.Http.OData.Extensions;
911
using System.Web.Http.OData.Query;
12+
using System.Web.Http.Results;
1013
using Microsoft.Data.OData;
1114
using NuGetGallery.Configuration;
1215
using NuGetGallery.OData.QueryFilter;
@@ -63,6 +66,17 @@ protected virtual string GetSiteRoot()
6366
return _configurationService.GetSiteRoot(UseHttps()).TrimEnd('/') + '/';
6467
}
6568

69+
protected ResponseMessageResult DeprecatedRequest(string message)
70+
{
71+
return ResponseMessage(Request.CreateErrorResponse(
72+
HttpStatusCode.BadRequest,
73+
new ODataError
74+
{
75+
ErrorCode = "NuGet.V2.Deprecated",
76+
Message = message,
77+
}));
78+
}
79+
6680
/// <summary>
6781
/// Generates a QueryResult.
6882
/// </summary>

tests/NuGetGallery.Facts/Controllers/ODataFeedControllerFactsBase.cs

+12
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.IO;
77
using System.Linq;
8+
using System.Net;
89
using System.Net.Http;
910
using System.Threading;
1011
using System.Threading.Tasks;
@@ -13,12 +14,14 @@
1314
using System.Web.Http.OData;
1415
using System.Web.Http.OData.Builder;
1516
using System.Web.Http.OData.Query;
17+
using System.Web.Http.Results;
1618
using Moq;
1719
using NuGet.Services.Entities;
1820
using NuGetGallery.Configuration;
1921
using NuGetGallery.Framework;
2022
using NuGetGallery.OData;
2123
using NuGetGallery.WebApi;
24+
using Xunit;
2225

2326
namespace NuGetGallery.Controllers
2427
{
@@ -49,6 +52,15 @@ protected ODataFeedControllerFactsBase()
4952
PackagesRepository = packagesRepositoryMock.Object;
5053
}
5154

55+
protected static async Task VerifyODataDeprecation(IHttpActionResult resultSet, string message)
56+
{
57+
var result = Assert.IsType<ResponseMessageResult>(resultSet);
58+
Assert.Equal(HttpStatusCode.BadRequest, result.Response.StatusCode);
59+
var content = await result.Response.Content.ReadAsStringAsync();
60+
Assert.Contains("NuGet.V2.Deprecated", content);
61+
Assert.Contains(message, content);
62+
}
63+
5264
protected abstract TController CreateController(
5365
IReadOnlyEntityRepository<Package> packagesRepository,
5466
IEntityRepository<Package> readWritePackagesRepository,

tests/NuGetGallery.Facts/Controllers/ODataV1FeedControllerFacts.cs

+11-9
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
using NuGetGallery.OData;
1111
using Moq;
1212
using Xunit;
13+
using System.Net;
14+
using System.Web.Http;
1315

1416
namespace NuGetGallery.Controllers
1517
{
@@ -42,7 +44,7 @@ public void Get_ReturnsBadRequestWhenOrderByInvalidColumn()
4244
}
4345

4446
[Fact]
45-
public void GetAll_ReturnsBadRequestWhenGetAllIsDisabled()
47+
public async Task GetAll_ReturnsBadRequestWhenGetAllIsDisabled()
4648
{
4749
// Arrange
4850
var featureFlagService = new Mock<IFeatureFlagService>();
@@ -55,12 +57,12 @@ public void GetAll_ReturnsBadRequestWhenGetAllIsDisabled()
5557
featureFlagService);
5658

5759
// Assert
58-
Assert.IsType<BadRequestErrorMessageResult>(resultSet);
60+
await VerifyODataDeprecation(resultSet, Strings.ODataDisabled);
5961
featureFlagService.Verify(x => x.IsODataV1GetAllEnabled());
6062
}
6163

6264
[Fact]
63-
public void GetAllCount_ReturnsBadRequestWhenGetAllIsDisabled()
65+
public async Task GetAllCount_ReturnsBadRequestWhenGetAllIsDisabled()
6466
{
6567
// Arrange
6668
var featureFlagService = new Mock<IFeatureFlagService>();
@@ -73,7 +75,7 @@ public void GetAllCount_ReturnsBadRequestWhenGetAllIsDisabled()
7375
featureFlagService);
7476

7577
// Assert
76-
Assert.IsType<BadRequestErrorMessageResult>(resultSet);
78+
await VerifyODataDeprecation(resultSet, Strings.ODataDisabled);
7779
featureFlagService.Verify(x => x.IsODataV1GetAllCountEnabled());
7880
}
7981

@@ -91,7 +93,7 @@ public async Task GetSpecific_ReturnsBadRequestNonHijackedIsDisabledAndQueryCann
9193
featureFlagService);
9294

9395
// Assert
94-
Assert.IsType<BadRequestErrorMessageResult>(resultSet);
96+
await VerifyODataDeprecation(resultSet, Strings.ODataParametersDisabled);
9597
featureFlagService.Verify(x => x.IsODataV1GetSpecificNonHijackedEnabled());
9698
}
9799

@@ -134,7 +136,7 @@ public async Task FindPackagesById_ReturnsBadRequestNonHijackedIsDisabledAndQuer
134136
featureFlagService);
135137

136138
// Assert
137-
Assert.IsType<BadRequestErrorMessageResult>(resultSet);
139+
await VerifyODataDeprecation(resultSet, Strings.ODataParametersDisabled);
138140
featureFlagService.Verify(x => x.IsODataV1FindPackagesByIdNonHijackedEnabled());
139141
}
140142

@@ -152,7 +154,7 @@ public async Task FindPackagesByIdCount_ReturnsBadRequestNonHijackedIsDisabledAn
152154
featureFlagService);
153155

154156
// Assert
155-
Assert.IsType<BadRequestErrorMessageResult>(resultSet);
157+
await VerifyODataDeprecation(resultSet, Strings.ODataParametersDisabled);
156158
featureFlagService.Verify(x => x.IsODataV1FindPackagesByIdCountNonHijackedEnabled());
157159
}
158160

@@ -195,7 +197,7 @@ public async Task Search_ReturnsBadRequestNonHijackedIsDisabledAndQueryCannotBeH
195197
featureFlagService);
196198

197199
// Assert
198-
Assert.IsType<BadRequestErrorMessageResult>(resultSet);
200+
await VerifyODataDeprecation(resultSet, Strings.ODataParametersDisabled);
199201
featureFlagService.Verify(x => x.IsODataV1SearchNonHijackedEnabled());
200202
}
201203

@@ -213,7 +215,7 @@ public async Task SearchCount_ReturnsBadRequestNonHijackedIsDisabledAndQueryCann
213215
featureFlagService);
214216

215217
// Assert
216-
Assert.IsType<BadRequestErrorMessageResult>(resultSet);
218+
await VerifyODataDeprecation(resultSet, Strings.ODataParametersDisabled);
217219
featureFlagService.Verify(x => x.IsODataV1SearchCountNonHijackedEnabled());
218220
}
219221

tests/NuGetGallery.Facts/Controllers/ODataV2FeedControllerFacts.cs

+7-7
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public async Task GetAll_ReturnsBadRequestNonHijackedIsDisabledAndQueryCannotBeH
7373
featureFlagService);
7474

7575
// Assert
76-
Assert.IsType<BadRequestErrorMessageResult>(resultSet);
76+
await VerifyODataDeprecation(resultSet, Strings.ODataParametersDisabled);
7777
featureFlagService.Verify(x => x.IsODataV2GetAllNonHijackedEnabled());
7878
}
7979

@@ -91,7 +91,7 @@ public async Task GetAllCount_ReturnsBadRequestNonHijackedIsDisabledAndQueryCann
9191
featureFlagService);
9292

9393
// Assert
94-
Assert.IsType<BadRequestErrorMessageResult>(resultSet);
94+
await VerifyODataDeprecation(resultSet, Strings.ODataParametersDisabled);
9595
featureFlagService.Verify(x => x.IsODataV2GetAllCountNonHijackedEnabled());
9696
}
9797

@@ -109,7 +109,7 @@ public async Task GetSpecific_ReturnsBadRequestNonHijackedIsDisabledAndQueryCann
109109
featureFlagService);
110110

111111
// Assert
112-
Assert.IsType<BadRequestErrorMessageResult>(resultSet);
112+
await VerifyODataDeprecation(resultSet, Strings.ODataParametersDisabled);
113113
featureFlagService.Verify(x => x.IsODataV2GetSpecificNonHijackedEnabled());
114114
}
115115

@@ -168,7 +168,7 @@ public async Task FindPackagesById_ReturnsBadRequestNonHijackedIsDisabledAndQuer
168168
featureFlagService);
169169

170170
// Assert
171-
Assert.IsType<BadRequestErrorMessageResult>(resultSet);
171+
await VerifyODataDeprecation(resultSet, Strings.ODataParametersDisabled);
172172
featureFlagService.Verify(x => x.IsODataV2FindPackagesByIdNonHijackedEnabled());
173173
}
174174

@@ -186,7 +186,7 @@ public async Task FindPackagesByIdCount_ReturnsBadRequestNonHijackedIsDisabledAn
186186
featureFlagService);
187187

188188
// Assert
189-
Assert.IsType<BadRequestErrorMessageResult>(resultSet);
189+
await VerifyODataDeprecation(resultSet, Strings.ODataParametersDisabled);
190190
featureFlagService.Verify(x => x.IsODataV2FindPackagesByIdCountNonHijackedEnabled());
191191
}
192192

@@ -249,7 +249,7 @@ public async Task Search_ReturnsBadRequestNonHijackedIsDisabledAndQueryCannotBeH
249249
featureFlagService);
250250

251251
// Assert
252-
Assert.IsType<BadRequestErrorMessageResult>(resultSet);
252+
await VerifyODataDeprecation(resultSet, Strings.ODataParametersDisabled);
253253
featureFlagService.Verify(x => x.IsODataV2SearchNonHijackedEnabled());
254254
}
255255

@@ -267,7 +267,7 @@ public async Task SearchCount_ReturnsBadRequestNonHijackedIsDisabledAndQueryCann
267267
featureFlagService);
268268

269269
// Assert
270-
Assert.IsType<BadRequestErrorMessageResult>(resultSet);
270+
await VerifyODataDeprecation(resultSet, Strings.ODataParametersDisabled);
271271
featureFlagService.Verify(x => x.IsODataV2SearchCountNonHijackedEnabled());
272272
}
273273

0 commit comments

Comments
 (0)