Skip to content

Commit 2d48281

Browse files
tyrielvCopilot
andcommitted
Combine auth initialization with config query
Add TryInitializeAndQueryGVFSConfig to GitAuthentication that merges the anonymous probe, credential fetch, and config query into a single flow, making at most 2 HTTP requests (or 1 for anonymous repos) and reusing the same TCP/TLS connection. Refactor TryInitialize to delegate to the combined method, eliminating the duplicated TryAnonymousQuery logic. Add TryAuthenticateAndQueryGVFSConfig to GVFSVerb and update CloneVerb, PrefetchVerb, and CacheServerVerb to use it, replacing the two-step TryAuthenticate + QueryGVFSConfig pattern with a single call. Remove the now-unused QueryGVFSConfigWithFallbackCacheServer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a917f38 commit 2d48281

6 files changed

Lines changed: 173 additions & 142 deletions

File tree

GVFS/GVFS.Common/Git/GitAuthentication.cs

Lines changed: 77 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -183,34 +183,98 @@ public bool TryGetCredentials(ITracer tracer, out string credentialString, out s
183183
return true;
184184
}
185185

186+
/// <summary>
187+
/// Initialize authentication by probing the server. Determines whether
188+
/// anonymous access is supported and, if not, fetches credentials.
189+
/// Callers that also need the GVFS config should use
190+
/// <see cref="TryInitializeAndQueryGVFSConfig"/> instead to avoid a
191+
/// redundant HTTP round-trip.
192+
/// </summary>
186193
public bool TryInitialize(ITracer tracer, Enlistment enlistment, out string errorMessage)
194+
{
195+
// Delegate to the combined method, discarding the config result.
196+
// This avoids duplicating the anonymous-probe + credential-fetch logic.
197+
return this.TryInitializeAndQueryGVFSConfig(
198+
tracer,
199+
enlistment,
200+
new RetryConfig(),
201+
out _,
202+
out errorMessage);
203+
}
204+
205+
/// <summary>
206+
/// Combines authentication initialization with the GVFS config query,
207+
/// eliminating a redundant HTTP round-trip. The anonymous probe and
208+
/// config query use the same request to /gvfs/config:
209+
/// 1. Config query → /gvfs/config → 200 (anonymous) or 401
210+
/// 2. If 401: credential fetch, then retry → 200
211+
/// This saves one HTTP request compared to probing auth separately
212+
/// and then querying config, and reuses the same TCP/TLS connection.
213+
/// </summary>
214+
public bool TryInitializeAndQueryGVFSConfig(
215+
ITracer tracer,
216+
Enlistment enlistment,
217+
RetryConfig retryConfig,
218+
out ServerGVFSConfig serverGVFSConfig,
219+
out string errorMessage)
187220
{
188221
if (this.isInitialized)
189222
{
190223
throw new InvalidOperationException("Already initialized");
191224
}
192225

226+
serverGVFSConfig = null;
193227
errorMessage = null;
194228

195-
bool isAnonymous;
196-
if (!this.TryAnonymousQuery(tracer, enlistment, out isAnonymous))
229+
using (ConfigHttpRequestor configRequestor = new ConfigHttpRequestor(tracer, enlistment, retryConfig))
197230
{
198-
errorMessage = $"Unable to determine if authentication is required";
199-
return false;
200-
}
231+
HttpStatusCode? httpStatus;
201232

202-
if (!isAnonymous &&
203-
!this.TryCallGitCredential(tracer, out errorMessage))
204-
{
233+
// First attempt without credentials. If anonymous access works,
234+
// we get the config in a single request.
235+
if (configRequestor.TryQueryGVFSConfig(false, out serverGVFSConfig, out httpStatus, out _))
236+
{
237+
this.IsAnonymous = true;
238+
this.isInitialized = true;
239+
tracer.RelatedInfo("{0}: Anonymous access succeeded, config obtained in one request", nameof(this.TryInitializeAndQueryGVFSConfig));
240+
return true;
241+
}
242+
243+
if (httpStatus != HttpStatusCode.Unauthorized)
244+
{
245+
errorMessage = "Unable to query /gvfs/config";
246+
tracer.RelatedWarning("{0}: Config query failed with status {1}", nameof(this.TryInitializeAndQueryGVFSConfig), httpStatus?.ToString() ?? "None");
247+
return false;
248+
}
249+
250+
// Server requires authentication — fetch credentials
251+
this.IsAnonymous = false;
252+
253+
if (!this.TryCallGitCredential(tracer, out errorMessage))
254+
{
255+
tracer.RelatedWarning("{0}: Credential fetch failed: {1}", nameof(this.TryInitializeAndQueryGVFSConfig), errorMessage);
256+
return false;
257+
}
258+
259+
this.isInitialized = true;
260+
261+
// Retry with credentials using the same ConfigHttpRequestor (reuses HttpClient/connection)
262+
if (configRequestor.TryQueryGVFSConfig(true, out serverGVFSConfig, out _, out errorMessage))
263+
{
264+
tracer.RelatedInfo("{0}: Config obtained with credentials", nameof(this.TryInitializeAndQueryGVFSConfig));
265+
return true;
266+
}
267+
268+
tracer.RelatedWarning("{0}: Config query failed with credentials: {1}", nameof(this.TryInitializeAndQueryGVFSConfig), errorMessage);
205269
return false;
206270
}
207-
208-
this.IsAnonymous = isAnonymous;
209-
this.isInitialized = true;
210-
return true;
211271
}
212272

213-
public bool TryInitializeAndRequireAuth(ITracer tracer, out string errorMessage)
273+
/// <summary>
274+
/// Test-only initialization that skips the network probe and goes
275+
/// straight to credential fetch. Not for production use.
276+
/// </summary>
277+
internal bool TryInitializeAndRequireAuth(ITracer tracer, out string errorMessage)
214278
{
215279
if (this.isInitialized)
216280
{
@@ -267,45 +331,6 @@ private static bool TryParseCredentialString(string credentialString, out string
267331
return false;
268332
}
269333

270-
private bool TryAnonymousQuery(ITracer tracer, Enlistment enlistment, out bool isAnonymous)
271-
{
272-
bool querySucceeded;
273-
using (ITracer anonymousTracer = tracer.StartActivity("AttemptAnonymousAuth", EventLevel.Informational))
274-
{
275-
HttpStatusCode? httpStatus;
276-
277-
using (ConfigHttpRequestor configRequestor = new ConfigHttpRequestor(anonymousTracer, enlistment, new RetryConfig()))
278-
{
279-
ServerGVFSConfig gvfsConfig;
280-
const bool LogErrors = false;
281-
if (configRequestor.TryQueryGVFSConfig(LogErrors, out gvfsConfig, out httpStatus, out _))
282-
{
283-
querySucceeded = true;
284-
isAnonymous = true;
285-
}
286-
else if (httpStatus == HttpStatusCode.Unauthorized)
287-
{
288-
querySucceeded = true;
289-
isAnonymous = false;
290-
}
291-
else
292-
{
293-
querySucceeded = false;
294-
isAnonymous = false;
295-
}
296-
}
297-
298-
anonymousTracer.Stop(new EventMetadata
299-
{
300-
{ "HttpStatus", httpStatus.HasValue ? ((int)httpStatus).ToString() : "None" },
301-
{ "QuerySucceeded", querySucceeded },
302-
{ "IsAnonymous", isAnonymous },
303-
});
304-
}
305-
306-
return querySucceeded;
307-
}
308-
309334
private DateTime GetNextAuthAttemptTime()
310335
{
311336
if (this.numberOfAttempts <= 1)

GVFS/GVFS.Mount/InProcessMount.cs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,21 +88,34 @@ public void Mount(EventLevel verbosity, Keywords keywords)
8888
// Start auth + config query immediately — these are network-bound and don't
8989
// depend on repo metadata or cache paths. Every millisecond of network latency
9090
// we can overlap with local I/O is a win.
91+
// TryInitializeAndQueryGVFSConfig combines the anonymous probe, credential fetch,
92+
// and config query into at most 2 HTTP requests (1 for anonymous repos), reusing
93+
// the same HttpClient/TCP connection.
9194
Stopwatch parallelTimer = Stopwatch.StartNew();
9295

9396
var networkTask = Task.Run(() =>
9497
{
9598
Stopwatch sw = Stopwatch.StartNew();
96-
string authError;
97-
if (!this.enlistment.Authentication.TryInitialize(this.tracer, this.enlistment, out authError))
99+
ServerGVFSConfig config;
100+
string authConfigError;
101+
102+
if (!this.enlistment.Authentication.TryInitializeAndQueryGVFSConfig(
103+
this.tracer, this.enlistment, this.retryConfig,
104+
out config, out authConfigError))
98105
{
99-
this.tracer.RelatedWarning("Mount will proceed, but new files cannot be accessed until GVFS can authenticate: " + authError);
106+
if (this.cacheServer != null && !string.IsNullOrWhiteSpace(this.cacheServer.Url))
107+
{
108+
this.tracer.RelatedWarning("Mount will proceed with fallback cache server: " + authConfigError);
109+
config = null;
110+
}
111+
else
112+
{
113+
this.FailMountAndExit("Unable to query /gvfs/config" + Environment.NewLine + authConfigError);
114+
}
100115
}
101116

102-
this.tracer.RelatedInfo("ParallelMount: Auth completed in {0}ms", sw.ElapsedMilliseconds);
103-
104-
ServerGVFSConfig config = this.QueryAndValidateGVFSConfig();
105-
this.tracer.RelatedInfo("ParallelMount: Auth + config query completed in {0}ms", sw.ElapsedMilliseconds);
117+
this.ValidateGVFSVersion(config);
118+
this.tracer.RelatedInfo("ParallelMount: Auth + config completed in {0}ms", sw.ElapsedMilliseconds);
106119
return config;
107120
});
108121

GVFS/GVFS/CommandLine/CacheServerVerb.cs

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -42,21 +42,19 @@ protected override void Execute(GVFSEnlistment enlistment)
4242

4343
using (ITracer tracer = new JsonTracer(GVFSConstants.GVFSEtwProviderName, "CacheVerb"))
4444
{
45-
string authErrorMessage;
46-
if (!this.TryAuthenticate(tracer, enlistment, out authErrorMessage))
47-
{
48-
this.ReportErrorAndExit(tracer, "Authentication failed: " + authErrorMessage);
49-
}
50-
5145
CacheServerResolver cacheServerResolver = new CacheServerResolver(tracer, enlistment);
5246
ServerGVFSConfig serverGVFSConfig = null;
5347
string error = null;
5448

5549
// Handle the three operation types: list, set, and get (default)
5650
if (this.ListCacheServers)
5751
{
58-
// For listing, require config endpoint to succeed
59-
serverGVFSConfig = this.QueryGVFSConfig(tracer, enlistment, retryConfig);
52+
// For listing, require config endpoint to succeed (no fallback)
53+
if (!this.TryAuthenticateAndQueryGVFSConfig(
54+
tracer, enlistment, retryConfig, out serverGVFSConfig, out error))
55+
{
56+
this.ReportErrorAndExit(tracer, "Unable to query /gvfs/config" + Environment.NewLine + error);
57+
}
6058

6159
List<CacheServerInfo> cacheServers = serverGVFSConfig.CacheServers.ToList();
6260

@@ -80,11 +78,12 @@ protected override void Execute(GVFSEnlistment enlistment)
8078
CacheServerInfo cacheServer = cacheServerResolver.ParseUrlOrFriendlyName(this.CacheToSet);
8179

8280
// For set operation, allow fallback if config endpoint fails but cache server URL is valid
83-
serverGVFSConfig = this.QueryGVFSConfigWithFallbackCacheServer(
84-
tracer,
85-
enlistment,
86-
retryConfig,
87-
cacheServer);
81+
if (!this.TryAuthenticateAndQueryGVFSConfig(
82+
tracer, enlistment, retryConfig, out serverGVFSConfig, out error,
83+
fallbackCacheServer: cacheServer))
84+
{
85+
this.ReportErrorAndExit(tracer, "Authentication failed: " + error);
86+
}
8887

8988
cacheServer = this.ResolveCacheServer(tracer, cacheServer, cacheServerResolver, serverGVFSConfig);
9089

@@ -101,11 +100,12 @@ protected override void Execute(GVFSEnlistment enlistment)
101100
CacheServerInfo cacheServer = CacheServerResolver.GetCacheServerFromConfig(enlistment);
102101

103102
// For get operation, allow fallback if config endpoint fails but cache server URL is valid
104-
serverGVFSConfig =this.QueryGVFSConfigWithFallbackCacheServer(
105-
tracer,
106-
enlistment,
107-
retryConfig,
108-
cacheServer);
103+
if (!this.TryAuthenticateAndQueryGVFSConfig(
104+
tracer, enlistment, retryConfig, out serverGVFSConfig, out error,
105+
fallbackCacheServer: cacheServer))
106+
{
107+
this.ReportErrorAndExit(tracer, "Authentication failed: " + error);
108+
}
109109

110110
CacheServerInfo resolvedCacheServer = cacheServerResolver.ResolveNameFromRemote(cacheServer.Url, serverGVFSConfig);
111111

GVFS/GVFS/CommandLine/CloneVerb.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -185,19 +185,19 @@ public override void Execute()
185185
this.Output.WriteLine(" Local Cache: " + resolvedLocalCacheRoot);
186186
this.Output.WriteLine(" Destination: " + enlistment.EnlistmentRoot);
187187

188-
string authErrorMessage;
189-
if (!this.TryAuthenticate(tracer, enlistment, out authErrorMessage))
190-
{
191-
this.ReportErrorAndExit(tracer, "Cannot clone because authentication failed: " + authErrorMessage);
192-
}
193-
194188
RetryConfig retryConfig = this.GetRetryConfig(tracer, enlistment, TimeSpan.FromMinutes(RetryConfig.FetchAndCloneTimeoutMinutes));
195189

196-
serverGVFSConfig = this.QueryGVFSConfigWithFallbackCacheServer(
190+
string authErrorMessage;
191+
if (!this.TryAuthenticateAndQueryGVFSConfig(
197192
tracer,
198193
enlistment,
199194
retryConfig,
200-
cacheServer);
195+
out serverGVFSConfig,
196+
out authErrorMessage,
197+
fallbackCacheServer: cacheServer))
198+
{
199+
this.ReportErrorAndExit(tracer, "Cannot clone because authentication failed: " + authErrorMessage);
200+
}
201201

202202
cacheServer = this.ResolveCacheServer(tracer, cacheServer, cacheServerResolver, serverGVFSConfig);
203203

GVFS/GVFS/CommandLine/GVFSVerb.cs

Lines changed: 40 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -429,14 +429,50 @@ protected bool ShowStatusWhileRunning(
429429

430430
protected bool TryAuthenticate(ITracer tracer, GVFSEnlistment enlistment, out string authErrorMessage)
431431
{
432-
string authError = null;
432+
return this.TryAuthenticateAndQueryGVFSConfig(tracer, enlistment, null, out _, out authErrorMessage);
433+
}
434+
435+
/// <summary>
436+
/// Combines authentication and GVFS config query into a single operation,
437+
/// eliminating a redundant HTTP round-trip. If <paramref name="retryConfig"/>
438+
/// is null, a default RetryConfig is used.
439+
/// If the config query fails but a valid <paramref name="fallbackCacheServer"/>
440+
/// URL is available, auth succeeds but <paramref name="serverGVFSConfig"/>
441+
/// will be null (caller should handle this gracefully).
442+
/// </summary>
443+
protected bool TryAuthenticateAndQueryGVFSConfig(
444+
ITracer tracer,
445+
GVFSEnlistment enlistment,
446+
RetryConfig retryConfig,
447+
out ServerGVFSConfig serverGVFSConfig,
448+
out string errorMessage,
449+
CacheServerInfo fallbackCacheServer = null)
450+
{
451+
ServerGVFSConfig config = null;
452+
string error = null;
433453

434454
bool result = this.ShowStatusWhileRunning(
435-
() => enlistment.Authentication.TryInitialize(tracer, enlistment, out authError),
455+
() => enlistment.Authentication.TryInitializeAndQueryGVFSConfig(
456+
tracer,
457+
enlistment,
458+
retryConfig ?? new RetryConfig(),
459+
out config,
460+
out error),
436461
"Authenticating",
437462
enlistment.EnlistmentRoot);
438463

439-
authErrorMessage = authError;
464+
if (!result && fallbackCacheServer != null && !string.IsNullOrWhiteSpace(fallbackCacheServer.Url))
465+
{
466+
// Auth/config query failed, but we have a fallback cache server.
467+
// Allow auth to succeed so mount/clone can proceed; config will be null.
468+
tracer.RelatedWarning("Config query failed but continuing with fallback cache server: " + error);
469+
serverGVFSConfig = null;
470+
errorMessage = null;
471+
return true;
472+
}
473+
474+
serverGVFSConfig = config;
475+
errorMessage = error;
440476
return result;
441477
}
442478

@@ -493,50 +529,7 @@ protected RetryConfig GetRetryConfig(ITracer tracer, GVFSEnlistment enlistment,
493529
return retryConfig;
494530
}
495531

496-
/// <summary>
497-
/// Attempts to query the GVFS config endpoint. If successful, returns the config.
498-
/// If the query fails but a valid fallback cache server URL is available, returns null and continues.
499-
/// (A warning will be logged later.)
500-
/// If the query fails and no valid fallback is available, reports an error and exits.
501-
/// </summary>
502-
protected ServerGVFSConfig QueryGVFSConfigWithFallbackCacheServer(
503-
ITracer tracer,
504-
GVFSEnlistment enlistment,
505-
RetryConfig retryConfig,
506-
CacheServerInfo fallbackCacheServer)
507-
{
508-
ServerGVFSConfig serverGVFSConfig = null;
509-
string errorMessage = null;
510-
bool configSuccess = this.ShowStatusWhileRunning(
511-
() =>
512-
{
513-
using (ConfigHttpRequestor configRequestor = new ConfigHttpRequestor(tracer, enlistment, retryConfig))
514-
{
515-
const bool LogErrors = true;
516-
return configRequestor.TryQueryGVFSConfig(LogErrors, out serverGVFSConfig, out _, out errorMessage);
517-
}
518-
},
519-
"Querying remote for config",
520-
suppressGvfsLogMessage: true);
521-
522-
if (!configSuccess)
523-
{
524-
// If a valid cache server URL is available, warn and continue
525-
if (fallbackCacheServer != null && !string.IsNullOrWhiteSpace(fallbackCacheServer.Url))
526-
{
527-
// Continue without config
528-
// Warning will be logged/displayed when version check is run
529-
return null;
530-
}
531-
else
532-
{
533-
this.ReportErrorAndExit(tracer, "Unable to query /gvfs/config" + Environment.NewLine + errorMessage);
534-
}
535-
}
536-
return serverGVFSConfig;
537-
}
538-
539-
// Restore original QueryGVFSConfig for other callers
532+
// QueryGVFSConfig for callers that require config to succeed (no fallback)
540533
protected ServerGVFSConfig QueryGVFSConfig(ITracer tracer, GVFSEnlistment enlistment, RetryConfig retryConfig)
541534
{
542535
ServerGVFSConfig serverGVFSConfig = null;

0 commit comments

Comments
 (0)