Skip to content

Commit 2e7d045

Browse files
paulirwinpaulirwin
committed
PR feedback and additional unit tests
1 parent a697291 commit 2e7d045

9 files changed

Lines changed: 768 additions & 148 deletions

File tree

‎Directory.Build.targets‎

Lines changed: 23 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@
3535
</PropertyGroup>
3636

3737
<PropertyGroup Label="Solution-level Publish to Project-specific Directory">
38-
<PublishDir Condition="'$(AlternatePublishRootDirectory)' != ''">
39-
$(AlternatePublishRootDirectory)/$(TargetFramework)/$(MSBuildProjectName)/</PublishDir>
38+
<PublishDir Condition="'$(AlternatePublishRootDirectory)' != ''">$(AlternatePublishRootDirectory)/$(TargetFramework)/$(MSBuildProjectName)/</PublishDir>
4039
</PropertyGroup>
4140

4241
<!-- Features in .NET 9.x+ only -->
@@ -51,6 +50,8 @@
5150

5251
<DefineConstants>$(DefineConstants);FEATURE_ASPNETCORE_TESTHOST</DefineConstants>
5352
<DefineConstants>$(DefineConstants);FEATURE_CANCELLATIONTOKENSOURCE_CANCELASYNC</DefineConstants>
53+
<DefineConstants>$(DefineConstants);FEATURE_HTTPCONTENT_READASSTREAM</DefineConstants>
54+
<DefineConstants>$(DefineConstants);FEATURE_HTTPCONTENT_READASSTREAM_CANCELLATIONTOKEN</DefineConstants>
5455

5556
</PropertyGroup>
5657

@@ -90,13 +91,6 @@
9091
<DebugType>portable</DebugType>
9192
</PropertyGroup>
9293

93-
<!-- Features in .NET 8.x+ only -->
94-
<PropertyGroup
95-
Condition=" $(TargetFramework.StartsWith('net8.')) Or $(TargetFramework.StartsWith('net9.')) Or $(TargetFramework.StartsWith('net10.')) ">
96-
<DefineConstants>$(DefineConstants);FEATURE_HTTPCONTENT_READASSTREAM</DefineConstants>
97-
<DefineConstants>$(DefineConstants);FEATURE_HTTPCONTENT_READASSTREAM_CANCELLATIONTOKEN</DefineConstants>
98-
</PropertyGroup>
99-
10094
<!-- Features in .NET Standard 2.1, .NET Core 3.x, and .NET 5.x+ only -->
10195
<PropertyGroup Condition=" '$(TargetFramework)' == 'netstandard2.1' Or $(TargetFramework.StartsWith('netcoreapp3.')) Or $(TargetFramework.StartsWith('net5.')) Or $(TargetFramework.StartsWith('net6.')) Or $(TargetFramework.StartsWith('net7.')) Or $(TargetFramework.StartsWith('net8.')) Or $(TargetFramework.StartsWith('net9.')) Or $(TargetFramework.StartsWith('net10.')) ">
10296

@@ -142,8 +136,7 @@
142136
</PropertyGroup>
143137

144138
<!-- Features in .NET Framework 4.5+ and .NET Standard 2.x only (No .NET Core support) -->
145-
<PropertyGroup
146-
Condition=" $(TargetFramework.StartsWith('net4')) Or $(TargetFramework.StartsWith('netstandard2.')) ">
139+
<PropertyGroup Condition=" $(TargetFramework.StartsWith('net4')) Or $(TargetFramework.StartsWith('netstandard2.')) ">
147140

148141
<!-- NOTE: The API for this exists in .NET Core, but it throws a PlatformNotSupportedException.
149142
We simply don't override this to get the same behavior. -->
@@ -152,8 +145,7 @@
152145
</PropertyGroup>
153146

154147
<!-- Features in .NET Framework 4.6.1+ only -->
155-
<PropertyGroup
156-
Condition="'$(TargetFramework)' == 'net461' Or '$(TargetFramework)' == 'net462' Or $(TargetFramework.StartsWith('net47')) Or $(TargetFramework.StartsWith('net48'))">
148+
<PropertyGroup Condition="'$(TargetFramework)' == 'net461' Or '$(TargetFramework)' == 'net462' Or $(TargetFramework.StartsWith('net47')) Or $(TargetFramework.StartsWith('net48'))">
157149

158150
<DefineConstants>$(DefineConstants);FEATURE_ICONFIGURATIONROOT_PROVIDERS</DefineConstants>
159151

@@ -163,8 +155,7 @@
163155
<PropertyGroup Condition="$(TargetFramework.StartsWith('net4'))">
164156

165157
<DefineConstants>$(DefineConstants);NETFRAMEWORK</DefineConstants>
166-
<!-- Although code access security is available in .NET Standard 2.0+ via platform extensions,
167-
we are excluding
158+
<!-- Although code access security is available in .NET Standard 2.0+ via platform extensions, we are excluding
168159
it due to the fact it is not a primary feature of Lucene.NET and it is supported in .NET Framework -->
169160
<DefineConstants>$(DefineConstants);FEATURE_CODE_ACCESS_SECURITY</DefineConstants>
170161
<DefineConstants>$(DefineConstants);FEATURE_MEMORYMAPPEDFILESECURITY</DefineConstants>
@@ -185,45 +176,34 @@
185176

186177
<PropertyGroup>
187178
<!-- NuGet.org only supports portable debug symbol format:
188-
https://docs.microsoft.com/en-us/nuget/create-packages/symbol-packages-snupkg#nugetorg-symbol-package-constraints -->
179+
https://docs.microsoft.com/en-us/nuget/create-packages/symbol-packages-snupkg#nugetorg-symbol-package-constraints -->
189180
<DebugType Condition=" '$(PortableDebugTypeOnly)' == 'true' ">portable</DebugType>
190181
</PropertyGroup>
191182

192-
<Target Name="AddInternalsVisibleTo" BeforeTargets="BeforeCompile"
193-
Label="Adds InternalsVisibleTo Attribute and PublicKey (if supplied)">
183+
<Target Name="AddInternalsVisibleTo" BeforeTargets="BeforeCompile" Label="Adds InternalsVisibleTo Attribute and PublicKey (if supplied)">
194184
<ItemGroup Condition="'@(InternalsVisibleTo->Count())' &gt; 0 ">
195185
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo">
196186
<_Parameter1>%(InternalsVisibleTo.Identity)</_Parameter1>
197-
<_Parameter1 Condition=" '$(SignAssembly)' == 'true' And '$(PublicKey)' != '' ">%(InternalsVisibleTo.Identity),
198-
PublicKey=$(PublicKey)</_Parameter1>
187+
<_Parameter1 Condition=" '$(SignAssembly)' == 'true' And '$(PublicKey)' != '' ">%(InternalsVisibleTo.Identity), PublicKey=$(PublicKey)</_Parameter1>
199188
</AssemblyAttribute>
200189
</ItemGroup>
201190
</Target>
202191

203-
<Target Name="PrintTargetFrameworks"
204-
Label="Prints the value for the $(TargetFrameworks) property or 'none' if no frameworks apply. Pass TestProjectsOnly=true to get results only if this is a test project.">
192+
<Target Name="PrintTargetFrameworks" Label="Prints the value for the $(TargetFrameworks) property or 'none' if no frameworks apply. Pass TestProjectsOnly=true to get results only if this is a test project.">
205193
<PropertyGroup>
206-
<DoOutputTargetFrameworks
207-
Condition=" '$(TestProjectsOnly)' != 'true' Or ('$(TestProjectsOnly)' == 'true' And '$(IsTestProject)' == 'true')">
208-
true</DoOutputTargetFrameworks>
209-
<OutputTargetFrameworks Condition=" '$(DoOutputTargetFrameworks)' == 'true' ">
210-
$(TargetFramework)</OutputTargetFrameworks>
194+
<DoOutputTargetFrameworks Condition=" '$(TestProjectsOnly)' != 'true' Or ('$(TestProjectsOnly)' == 'true' And '$(IsTestProject)' == 'true')">true</DoOutputTargetFrameworks>
195+
<OutputTargetFrameworks Condition=" '$(DoOutputTargetFrameworks)' == 'true' ">$(TargetFramework)</OutputTargetFrameworks>
211196
<!-- Fallback to TargetFrameworks field if TargetFramework is empty -->
212-
<OutputTargetFrameworks
213-
Condition=" '$(DoOutputTargetFrameworks)' == 'true' And '$(OutputTargetFrameworks)' == '' ">
214-
$(TargetFrameworks)</OutputTargetFrameworks>
197+
<OutputTargetFrameworks Condition=" '$(DoOutputTargetFrameworks)' == 'true' And '$(OutputTargetFrameworks)' == '' ">$(TargetFrameworks)</OutputTargetFrameworks>
215198
<OutputTargetFrameworks Condition=" '$(OutputTargetFrameworks)' == '' ">none</OutputTargetFrameworks>
216199
</PropertyGroup>
217200
<Message Text="SupportedTargetFrameworks=$(OutputTargetFrameworks)" Importance="high"/>
218201
</Target>
219202

220203

221-
<!-- Disable Json Source Generator from being added to projects. This is being done to work around
222-
a bug in VS 2022
223-
which appeared around VS2022 7.3.6 which causes StackOverflowExceptions when compiling the
224-
solution.
225-
see:
226-
https://developercommunity.visualstudio.com/t/VS-2022-1736-Process-is-terminated-due/10173885#T-ND10184855
204+
<!-- Disable Json Source Generator from being added to projects. This is being done to work around a bug in VS 2022
205+
which appeared around VS2022 7.3.6 which causes StackOverflowExceptions when compiling the solution.
206+
see: https://developercommunity.visualstudio.com/t/VS-2022-1736-Process-is-terminated-due/10173885#T-ND10184855
227207
for more details. Once a fix is rolled out for VS2022, this block can be removed -->
228208
<Target Name="RemoveJsonSourceGenerator" BeforeTargets="CoreCompile">
229209
<ItemGroup>
@@ -234,33 +214,25 @@
234214
</ItemGroup>
235215

236216
<ItemGroup>
237-
<AnalyzersToRemove Include="@(AnalyzersByFileName)"
238-
Condition="'@(AnalyzersToRemoveByFileName)' == '@(AnalyzersByFileName)' and '%(Identity)' != ''" />
217+
<AnalyzersToRemove Include="@(AnalyzersByFileName)" Condition="'@(AnalyzersToRemoveByFileName)' == '@(AnalyzersByFileName)' and '%(Identity)' != ''" />
239218
<Analyzer Remove="%(AnalyzersToRemove.OriginalIdentity)" />
240219
</ItemGroup>
241220
</Target>
242221

243222

244223
<!-- Global PackageReferences -->
245224
<ItemGroup>
246-
<!-- This is to allow the .NET Framework references to be machine-indepenedent so builds can
247-
happen without installing prerequisites -->
248-
<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies"
249-
Version="$(MicrosoftNETFrameworkReferenceAssembliesPackageReferenceVersion)"
250-
PrivateAssets="All" />
225+
<!-- This is to allow the .NET Framework references to be machine-indepenedent so builds can happen without installing prerequisites -->
226+
<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="$(MicrosoftNETFrameworkReferenceAssembliesPackageReferenceVersion)" PrivateAssets="All" />
251227
</ItemGroup>
252228

253229
<!-- This is for testing only, we use SourceLink from any Azure DevOps git repo -->
254-
<ItemGroup Condition=" '$(BUILD_REPOSITORY_PROVIDER)' == 'TfsGit' "
255-
Label="SourceLink Packages (experimental Azure Repos)">
256-
<PackageReference Include="Microsoft.SourceLink.AzureRepos.Git"
257-
Version="$(MicrosoftSourceLinkAzureReposGitPackageReferenceVersion)" PrivateAssets="All" />
230+
<ItemGroup Condition=" '$(BUILD_REPOSITORY_PROVIDER)' == 'TfsGit' " Label="SourceLink Packages (experimental Azure Repos)">
231+
<PackageReference Include="Microsoft.SourceLink.AzureRepos.Git" Version="$(MicrosoftSourceLinkAzureReposGitPackageReferenceVersion)" PrivateAssets="All"/>
258232
</ItemGroup>
259233

260-
<ItemGroup Condition=" '$(BUILD_REPOSITORY_PROVIDER)' == 'GitHub' "
261-
Label="SourceLink Packages (main repo)">
262-
<PackageReference Include="Microsoft.SourceLink.GitHub"
263-
Version="$(MicrosoftSourceLinkGitHubPackageReferenceVersion)" PrivateAssets="All" />
234+
<ItemGroup Condition=" '$(BUILD_REPOSITORY_PROVIDER)' == 'GitHub' " Label="SourceLink Packages (main repo)">
235+
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="$(MicrosoftSourceLinkGitHubPackageReferenceVersion)" PrivateAssets="All"/>
264236
</ItemGroup>
265237

266238
<ItemGroup>

‎src/Lucene.Net.Replicator/Http/HttpClientBase.cs‎

Lines changed: 16 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ protected virtual HttpResponseMessage ExecutePost(string request, HttpContent co
194194
/// <b>Internal:</b> Execute a POST request asynchronously with custom HttpContent.
195195
/// The <paramref name="parameters"/> argument is treated as: name1,value1,name2,value2,...
196196
/// </summary>
197-
protected virtual async Task<HttpResponseMessage> ExecutePostAsync(string request, HttpContent content, params string[]? parameters)
197+
protected virtual async Task<HttpResponseMessage> ExecutePostAsync(string request, HttpContent content, CancellationToken cancellationToken, params string[]? parameters)
198198
{
199199
EnsureOpen();
200200

@@ -203,9 +203,7 @@ protected virtual async Task<HttpResponseMessage> ExecutePostAsync(string reques
203203
Content = content
204204
};
205205

206-
var resp = await httpc.SendAsync(req).ConfigureAwait(false); // Async call
207-
VerifyStatus(resp);
208-
return resp;
206+
return await ExecuteAsync(req, cancellationToken).ConfigureAwait(false);
209207
}
210208

211209

@@ -225,31 +223,11 @@ protected virtual HttpResponseMessage ExecuteGet(string request, params string[]
225223
/// <summary>
226224
/// Execute a GET request asynchronously with an array of parameters.
227225
/// </summary>
228-
protected Task<HttpResponseMessage> ExecuteGetAsync(string action, string[]? parameters, CancellationToken cancellationToken)
226+
protected virtual async Task<HttpResponseMessage> ExecuteGetAsync(string action, string[]? parameters, CancellationToken cancellationToken)
229227
{
230228
EnsureOpen();
231-
var url = QueryString(action, parameters);
232-
return httpc.GetAsync(url, cancellationToken);
233-
}
234-
235-
/// <summary>
236-
/// Execute a GET request asynchronously with up to 3 name/value parameters.
237-
/// </summary>
238-
protected Task<HttpResponseMessage> ExecuteGetAsync(
239-
string action,
240-
string param1, string value1,
241-
string? param2 = null, string? value2 = null,
242-
string? param3 = null, string? value3 = null,
243-
CancellationToken cancellationToken = default)
244-
{
245-
EnsureOpen();
246-
var url = (param2 == null && param3 == null)
247-
? QueryString(action, param1, value1)
248-
: QueryString(action,
249-
param1, value1,
250-
param2 ?? string.Empty, value2 ?? string.Empty,
251-
param3 ?? string.Empty, value3 ?? string.Empty);
252-
return httpc.GetAsync(url, cancellationToken);
229+
var req = new HttpRequestMessage(HttpMethod.Get, QueryString(action, parameters));
230+
return await ExecuteAsync(req, cancellationToken).ConfigureAwait(false);
253231
}
254232

255233
private HttpResponseMessage Execute(HttpRequestMessage request)
@@ -261,6 +239,16 @@ private HttpResponseMessage Execute(HttpRequestMessage request)
261239
return response;
262240
}
263241

242+
// LUCENENET specific - async counterpart to Execute used by ExecuteGetAsync/ExecutePostAsync.
243+
// Uses ResponseHeadersRead so callers can stream large response bodies (e.g. replication files)
244+
// rather than buffering them into memory, and verifies status (matching sync Execute).
245+
private async Task<HttpResponseMessage> ExecuteAsync(HttpRequestMessage request, CancellationToken cancellationToken)
246+
{
247+
var response = await httpc.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false);
248+
VerifyStatus(response);
249+
return response;
250+
}
251+
264252
private string QueryString(string request, params string[]? parameters)
265253
{
266254
return parameters is null || parameters.Length == 0
@@ -446,7 +434,6 @@ protected virtual async Task<T> DoActionAsync<T>(HttpResponseMessage response, b
446434
Exception? th /* = null */;
447435
try
448436
{
449-
VerifyStatus(response);
450437
return await call().ConfigureAwait(false);
451438
}
452439
catch (Exception t) when (t.IsThrowable())
@@ -463,14 +450,7 @@ protected virtual async Task<T> DoActionAsync<T>(HttpResponseMessage response, b
463450
{
464451
if (consume)
465452
{
466-
try
467-
{
468-
ConsumeQuietly(response);
469-
}
470-
catch
471-
{
472-
// ignore on purpose
473-
}
453+
ConsumeQuietly(response);
474454
}
475455
}
476456
}

‎src/Lucene.Net.Replicator/Http/HttpReplicator.cs‎

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -158,28 +158,38 @@ public async Task<Stream> ObtainFileAsync(string sessionId, string source, strin
158158
{
159159
var response = await ExecuteGetAsync(
160160
nameof(ReplicationService.ReplicationAction.OBTAIN),
161-
ReplicationService.REPLICATE_SESSION_ID_PARAM, sessionId,
162-
ReplicationService.REPLICATE_SOURCE_PARAM, source,
163-
ReplicationService.REPLICATE_FILENAME_PARAM, fileName,
164-
cancellationToken: cancellationToken).ConfigureAwait(false);
165-
166-
return await DoActionAsync(response, false,
167-
async () => await GetResponseStreamWithOwnershipAsync(response, cancellationToken).ConfigureAwait(false))
168-
.ConfigureAwait(false);
161+
new[]
162+
{
163+
ReplicationService.REPLICATE_SESSION_ID_PARAM, sessionId,
164+
ReplicationService.REPLICATE_SOURCE_PARAM, source,
165+
ReplicationService.REPLICATE_FILENAME_PARAM, fileName,
166+
},
167+
cancellationToken).ConfigureAwait(false);
168+
169+
// On success, ownership of the response transfers to the returned stream
170+
// (which disposes the response when the stream is disposed). On failure,
171+
// we must dispose the response ourselves.
172+
try
173+
{
174+
return await DoActionAsync(response, false,
175+
async () => await GetResponseStreamWithOwnershipAsync(response, cancellationToken).ConfigureAwait(false))
176+
.ConfigureAwait(false);
177+
}
178+
catch
179+
{
180+
response.Dispose();
181+
throw;
182+
}
169183
}
170184

171185
/// <summary>
172-
/// Publishes a new <see cref="IRevision"/> asynchronously.
173-
/// Not supported in this implementation.
186+
/// Not supported.
174187
/// </summary>
175-
/// <param name="revision">The revision to publish.</param>
176-
/// <param name="cancellationToken">Cancellation token.</param>
177-
/// <returns>A <see cref="Task"/> representing the operation.</returns>
178-
/// <exception cref="NotSupportedException">Always thrown.</exception>
188+
/// <exception cref="NotSupportedException">this replicator implementation does not support remote publishing of revisions</exception>
179189
public Task PublishAsync(IRevision revision, CancellationToken cancellationToken = default)
180190
{
181-
throw UnsupportedOperationException.Create(
182-
"this replicator implementation does not support remote publishing of revisions");
191+
return Task.FromException(UnsupportedOperationException.Create(
192+
"this replicator implementation does not support remote publishing of revisions"));
183193
}
184194

185195
/// <summary>
@@ -192,8 +202,8 @@ public async Task ReleaseAsync(string sessionId, CancellationToken cancellationT
192202
{
193203
using var response = await ExecuteGetAsync(
194204
nameof(ReplicationService.ReplicationAction.RELEASE),
195-
ReplicationService.REPLICATE_SESSION_ID_PARAM, sessionId,
196-
cancellationToken: cancellationToken).ConfigureAwait(false);
205+
new[] { ReplicationService.REPLICATE_SESSION_ID_PARAM, sessionId },
206+
cancellationToken).ConfigureAwait(false);
197207

198208
await DoActionAsync(response, () =>
199209
{

‎src/Lucene.Net.Replicator/IAsyncReplicator.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ public interface IAsyncReplicator : IReplicator
3333
Task ReleaseAsync(string sessionId, CancellationToken cancellationToken = default);
3434

3535
/// <summary>
36-
/// Publishing revisions is not supported in HttpReplicator; throw if called.
36+
/// Publish a new <see cref="IRevision"/> for consumption by clients asynchronously.
37+
/// See <see cref="IReplicator.Publish(IRevision)"/> for details.
3738
/// </summary>
3839
Task PublishAsync(IRevision revision, CancellationToken cancellationToken = default);
3940
}
4041
}
41-
#nullable restore

0 commit comments

Comments
 (0)