Merge pull request #29656 from peppy/provide-api-earlier

Provide API context earlier to api requests in order to fix missing schedules

osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs

@@ -257,7 +257,7 @@ namespace osu.Game.Tests.Online
             {
             }
 
-            protected override string Target => null;
+            protected override string Target => string.Empty;
         }
     }
 }

osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs

@@ -446,7 +446,7 @@ namespace osu.Game.Tests.Visual.Online
         {
             AddStep("Show overlay with channel 1", () =>
             {
-                channelManager.JoinChannel(testChannel1);
+                channelManager.CurrentChannel.Value = channelManager.JoinChannel(testChannel1);
                 chatOverlay.Show();
             });
             waitForChannel1Visible();
@@ -462,7 +462,7 @@ namespace osu.Game.Tests.Visual.Online
         {
             AddStep("Show overlay with channel 1", () =>
             {
-                channelManager.JoinChannel(testChannel1);
+                channelManager.CurrentChannel.Value = channelManager.JoinChannel(testChannel1);
                 chatOverlay.Show();
             });
             waitForChannel1Visible();

osu.Game/Online/API/APIAccess.cs

@@ -159,7 +159,7 @@ namespace osu.Game.Online.API
 
         private void onTokenChanged(ValueChangedEvent<OAuthToken> e) => config.SetValue(OsuSetting.Token, config.Get<bool>(OsuSetting.SavePassword) ? authentication.TokenString : string.Empty);
 
-        internal new void Schedule(Action action) => base.Schedule(action);
+        void IAPIProvider.Schedule(Action action) => base.Schedule(action);
 
         public string AccessToken => authentication.RequestAccessToken();
 
@@ -385,7 +385,8 @@ namespace osu.Game.Online.API
         {
             try
             {
-                request.Perform(this);
+                request.AttachAPI(this);
+                request.Perform();
             }
             catch (Exception e)
             {
@@ -483,7 +484,8 @@ namespace osu.Game.Online.API
         {
             try
             {
-                req.Perform(this);
+                req.AttachAPI(this);
+                req.Perform();
 
                 if (req.CompletionState != APIRequestCompletionState.Completed)
                     return false;
@@ -568,6 +570,8 @@ namespace osu.Game.Online.API
         {
             lock (queue)
             {
+                request.AttachAPI(this);
+
                 if (state.Value == APIState.Offline)
                 {
                     request.Fail(new WebException(@"User not logged in"));

osu.Game/Online/API/APIDownloadRequest.cs

@@ -4,6 +4,7 @@
 #nullable disable
 
 using System;
+using System.Diagnostics;
 using System.IO;
 using osu.Framework.IO.Network;
 
@@ -34,7 +35,11 @@ namespace osu.Game.Online.API
             return request;
         }
 
-        private void request_Progress(long current, long total) => API.Schedule(() => Progressed?.Invoke(current, total));
+        private void request_Progress(long current, long total)
+        {
+            Debug.Assert(API != null);
+            API.Schedule(() => Progressed?.Invoke(current, total));
+        }
 
         protected void TriggerSuccess(string filename)
         {

osu.Game/Online/API/APIRequest.cs

@@ -1,11 +1,9 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-#nullable disable
-
 using System;
+using System.Diagnostics;
 using System.Globalization;
-using JetBrains.Annotations;
 using Newtonsoft.Json;
 using osu.Framework.Extensions.TypeExtensions;
 using osu.Framework.IO.Network;
@@ -26,18 +24,17 @@ namespace osu.Game.Online.API
         /// <summary>
         /// The deserialised response object. May be null if the request or deserialisation failed.
         /// </summary>
-        [CanBeNull]
-        public T Response { get; private set; }
+        public T? Response { get; private set; }
 
         /// <summary>
         /// Invoked on successful completion of an API request.
         /// This will be scheduled to the API's internal scheduler (run on update thread automatically).
         /// </summary>
-        public new event APISuccessHandler<T> Success;
+        public new event APISuccessHandler<T>? Success;
 
         protected APIRequest()
         {
-            base.Success += () => Success?.Invoke(Response);
+            base.Success += () => Success?.Invoke(Response!);
         }
 
         protected override void PostProcess()
@@ -71,27 +68,28 @@ namespace osu.Game.Online.API
 
         protected virtual WebRequest CreateWebRequest() => new OsuWebRequest(Uri);
 
-        protected virtual string Uri => $@"{API.APIEndpointUrl}/api/v2/{Target}";
+        protected virtual string Uri => $@"{API!.APIEndpointUrl}/api/v2/{Target}";
 
-        protected APIAccess API;
-        protected WebRequest WebRequest;
+        protected IAPIProvider? API;
+
+        protected WebRequest? WebRequest;
 
         /// <summary>
         /// The currently logged in user. Note that this will only be populated during <see cref="Perform"/>.
         /// </summary>
-        protected APIUser User { get; private set; }
+        protected APIUser? User { get; private set; }
 
         /// <summary>
         /// Invoked on successful completion of an API request.
         /// This will be scheduled to the API's internal scheduler (run on update thread automatically).
         /// </summary>
-        public event APISuccessHandler Success;
+        public event APISuccessHandler? Success;
 
         /// <summary>
         /// Invoked on failure to complete an API request.
         /// This will be scheduled to the API's internal scheduler (run on update thread automatically).
         /// </summary>
-        public event APIFailureHandler Failure;
+        public event APIFailureHandler? Failure;
 
         private readonly object completionStateLock = new object();
 
@@ -101,16 +99,29 @@ namespace osu.Game.Online.API
         /// </summary>
         public APIRequestCompletionState CompletionState { get; private set; }
 
-        public void Perform(IAPIProvider api)
+        /// <summary>
+        /// Should be called before <see cref="Perform"/> to give API context.
+        /// </summary>
+        /// <remarks>
+        /// This allows scheduling of operations back to the correct thread (which may be required before <see cref="Perform"/> is called).
+        /// </remarks>
+        public void AttachAPI(IAPIProvider apiAccess)
         {
-            if (!(api is APIAccess apiAccess))
+            if (API != null && API != apiAccess)
+                throw new InvalidOperationException("Attached API cannot be changed after initial set.");
+
+            API = apiAccess;
+        }
+
+        public void Perform()
+        {
+            if (API == null)
             {
                 Fail(new NotSupportedException($"A {nameof(APIAccess)} is required to perform requests."));
                 return;
             }
 
-            API = apiAccess;
-            User = apiAccess.LocalUser.Value;
+            User = API.LocalUser.Value;
 
             if (isFailing) return;
 
@@ -153,6 +164,8 @@ namespace osu.Game.Online.API
 
         internal void TriggerSuccess()
         {
+            Debug.Assert(API != null);
+
             lock (completionStateLock)
             {
                 if (CompletionState != APIRequestCompletionState.Waiting)
@@ -161,14 +174,13 @@ namespace osu.Game.Online.API
                 CompletionState = APIRequestCompletionState.Completed;
             }
 
-            if (API == null)
-                Success?.Invoke();
-            else
-                API.Schedule(() => Success?.Invoke());
+            API.Schedule(() => Success?.Invoke());
         }
 
         internal void TriggerFailure(Exception e)
         {
+            Debug.Assert(API != null);
+
             lock (completionStateLock)
             {
                 if (CompletionState != APIRequestCompletionState.Waiting)
@@ -177,10 +189,7 @@ namespace osu.Game.Online.API
                 CompletionState = APIRequestCompletionState.Failed;
             }
 
-            if (API == null)
-                Failure?.Invoke(e);
-            else
-                API.Schedule(() => Failure?.Invoke(e));
+            API.Schedule(() => Failure?.Invoke(e));
         }
 
         public void Cancel() => Fail(new OperationCanceledException(@"Request cancelled"));
@@ -197,7 +206,7 @@ namespace osu.Game.Online.API
                 // in the case of a cancellation we don't care about whether there's an error in the response.
                 if (!(e is OperationCanceledException))
                 {
-                    string responseString = WebRequest?.GetResponseString();
+                    string? responseString = WebRequest?.GetResponseString();
 
                     // naive check whether there's an error in the response to avoid unnecessary JSON deserialisation.
                     if (!string.IsNullOrEmpty(responseString) && responseString.Contains(@"""error"""))
@@ -235,7 +244,7 @@ namespace osu.Game.Online.API
         private class DisplayableError
         {
             [JsonProperty("error")]
-            public string ErrorMessage { get; set; }
+            public string ErrorMessage { get; set; } = string.Empty;
         }
     }
 

osu.Game/Online/API/DummyAPIAccess.cs

@@ -82,6 +82,8 @@ namespace osu.Game.Online.API
 
         public virtual void Queue(APIRequest request)
         {
+            request.AttachAPI(this);
+
             Schedule(() =>
             {
                 if (HandleRequest?.Invoke(request) != true)
@@ -98,10 +100,17 @@ namespace osu.Game.Online.API
             });
         }
 
-        public void Perform(APIRequest request) => HandleRequest?.Invoke(request);
+        void IAPIProvider.Schedule(Action action) => base.Schedule(action);
+
+        public void Perform(APIRequest request)
+        {
+            request.AttachAPI(this);
+            HandleRequest?.Invoke(request);
+        }
 
         public Task PerformAsync(APIRequest request)
         {
+            request.AttachAPI(this);
             HandleRequest?.Invoke(request);
             return Task.CompletedTask;
         }
@@ -155,6 +164,8 @@ namespace osu.Game.Online.API
             state.Value = APIState.Connecting;
             LastLoginError = null;
 
+            request.AttachAPI(this);
+
             // if no handler installed / handler can't handle verification, just assume that the server would verify for simplicity.
             if (HandleRequest?.Invoke(request) != true)
                 onSuccessfulLogin();

osu.Game/Online/API/IAPIProvider.cs

@@ -134,6 +134,11 @@ namespace osu.Game.Online.API
         /// </summary>
         void UpdateStatistics(UserStatistics newStatistics);
 
+        /// <summary>
+        /// Schedule a callback to run on the update thread.
+        /// </summary>
+        internal void Schedule(Action action);
+
         /// <summary>
         /// Constructs a new <see cref="IHubClientConnector"/>. May be null if not supported.
         /// </summary>

osu.Game/Online/API/Requests/JoinChannelRequest.cs

@@ -23,6 +23,6 @@ namespace osu.Game.Online.API.Requests
             return req;
         }
 
-        protected override string Target => $@"chat/channels/{channel.Id}/users/{User.Id}";
+        protected override string Target => $@"chat/channels/{channel.Id}/users/{User!.Id}";
     }
 }

osu.Game/Online/API/Requests/LeaveChannelRequest.cs

@@ -23,6 +23,6 @@ namespace osu.Game.Online.API.Requests
             return req;
         }
 
-        protected override string Target => $@"chat/channels/{channel.Id}/users/{User.Id}";
+        protected override string Target => $@"chat/channels/{channel.Id}/users/{User!.Id}";
     }
 }

osu.Game/Online/Chat/WebSocketChatClient.cs

@@ -80,7 +80,7 @@ namespace osu.Game.Online.Chat
 
             fetchReq.Success += updates =>
             {
-                if (updates?.Presence != null)
+                if (updates.Presence != null)
                 {
                     foreach (var channel in updates.Presence)
                         joinChannel(channel);

osu.Game/Online/Rooms/JoinRoomRequest.cs

@@ -27,6 +27,6 @@ namespace osu.Game.Online.Rooms
             return req;
         }
 
-        protected override string Target => $@"rooms/{Room.RoomID.Value}/users/{User.Id}";
+        protected override string Target => $@"rooms/{Room.RoomID.Value}/users/{User!.Id}";
     }
 }

osu.Game/Online/Rooms/PartRoomRequest.cs

@@ -23,6 +23,6 @@ namespace osu.Game.Online.Rooms
             return req;
         }
 
-        protected override string Target => $"rooms/{room.RoomID.Value}/users/{User.Id}";
+        protected override string Target => $"rooms/{room.RoomID.Value}/users/{User!.Id}";
     }
 }

osu.Game/OsuGameBase.cs

@@ -692,7 +692,7 @@ namespace osu.Game
             if (Interlocked.Decrement(ref allowableExceptions) < 0)
             {
                 Logger.Log("Too many unhandled exceptions, crashing out.");
-                RulesetStore.TryDisableCustomRulesetsCausing(ex);
+                RulesetStore?.TryDisableCustomRulesetsCausing(ex);
                 return false;
             }
 

osu.Game/Tests/PollingChatClient.cs

@@ -48,7 +48,7 @@ namespace osu.Game.Tests
 
             fetchReq.Success += updates =>
             {
-                if (updates?.Presence != null)
+                if (updates.Presence != null)
                 {
                     foreach (var channel in updates.Presence)
                         handleChannelJoined(channel);