From 7558d91bf97b1d87c27fdf1a6eeaafa93ab21f35 Mon Sep 17 00:00:00 2001 From: Bailey Date: Sat, 11 Nov 2023 06:30:24 -0600 Subject: [PATCH] [552] fix ArgumentNullException in SyncService when no recent workouts found (#557) * [552] fix ArgumentNullException in SyncService when no recent workouts found * version bump for release --- .vscode/tasks.json | 41 ++++++ src/Common/Constants.cs | 2 +- src/Sync/SyncService.cs | 192 ++++++++++++------------- src/UnitTests/Sync/SyncServiceTests.cs | 49 +++++++ vNextReleaseNotes.md | 20 +-- 5 files changed, 189 insertions(+), 115 deletions(-) create mode 100644 .vscode/tasks.json diff --git a/.vscode/tasks.json b/.vscode/tasks.json new file mode 100644 index 000000000..09486d2c4 --- /dev/null +++ b/.vscode/tasks.json @@ -0,0 +1,41 @@ +{ + "version": "2.0.0", + "tasks": [ + { + "label": "build", + "command": "dotnet", + "type": "process", + "args": [ + "build", + "${workspaceFolder}/PelotonToGarmin.sln", + "/property:GenerateFullPaths=true", + "/consoleloggerparameters:NoSummary;ForceNoAlign" + ], + "problemMatcher": "$msCompile" + }, + { + "label": "publish", + "command": "dotnet", + "type": "process", + "args": [ + "publish", + "${workspaceFolder}/PelotonToGarmin.sln", + "/property:GenerateFullPaths=true", + "/consoleloggerparameters:NoSummary;ForceNoAlign" + ], + "problemMatcher": "$msCompile" + }, + { + "label": "watch", + "command": "dotnet", + "type": "process", + "args": [ + "watch", + "run", + "--project", + "${workspaceFolder}/PelotonToGarmin.sln" + ], + "problemMatcher": "$msCompile" + } + ] +} \ No newline at end of file diff --git a/src/Common/Constants.cs b/src/Common/Constants.cs index 0a8a0ff52..c31a41cc9 100644 --- a/src/Common/Constants.cs +++ b/src/Common/Constants.cs @@ -7,6 +7,6 @@ public static class Constants public const string ConsoleAppName = "p2g_console"; public const string ApiName = "p2g_api"; public const string WebUIName = "p2g_webui"; - public const string AppVersion = "3.6.0"; + public const string AppVersion = "3.6.1"; } } diff --git a/src/Sync/SyncService.cs b/src/Sync/SyncService.cs index f7bc15fac..e152a576d 100644 --- a/src/Sync/SyncService.cs +++ b/src/Sync/SyncService.cs @@ -3,11 +3,11 @@ using Common.Dto; using Common.Dto.Peloton; using Common.Observe; -using Common.Service; +using Common.Service; using Common.Stateful; using Conversion; using Garmin; -using Garmin.Auth; +using Garmin.Auth; using Peloton; using Prometheus; using Serilog; @@ -33,7 +33,7 @@ public class SyncService : ISyncService private readonly IGarminUploader _garminUploader; private readonly IEnumerable _converters; private readonly ISyncStatusDb _db; - private readonly IFileHandling _fileHandler; + private readonly IFileHandling _fileHandler; private readonly ISettingsService _settingsService; public SyncService(ISettingsService settingService, IPelotonService pelotonService, IGarminUploader garminUploader, IEnumerable converters, ISyncStatusDb dbClient, IFileHandling fileHandler) @@ -50,9 +50,9 @@ public async Task SyncAsync(int numWorkouts) { using var timer = SyncHistogram.NewTimer(); using var activity = Tracing.Trace($"{nameof(SyncService)}.{nameof(SyncAsync)}.ByNumWorkouts") - .WithTag("numWorkouts", numWorkouts.ToString()); - - var settings = await _settingsService.GetSettingsAsync(); + .WithTag("numWorkouts", numWorkouts.ToString()); + + var settings = await _settingsService.GetSettingsAsync(); return await SyncWithWorkoutLoaderAsync(() => _pelotonService.GetRecentWorkoutsAsync(numWorkouts), settings.Peloton.ExcludeWorkoutTypes); } @@ -62,17 +62,17 @@ public async Task SyncAsync(IEnumerable workoutIds, ICollect using var activity = Tracing.Trace($"{nameof(SyncService)}.{nameof(SyncAsync)}.ByWorkoutIds"); var response = new SyncResult(); - var recentWorkouts = workoutIds.Select(w => new Workout() { Id = w }).ToList(); + var recentWorkouts = workoutIds.Select(w => new Workout() { Id = w }).ToList(); var settings = await _settingsService.GetSettingsAsync(); UserData? userData = null; - try - { - userData = await _pelotonService.GetUserDataAsync(); - } - catch (Exception e) + try { - _logger.Warning(e, $"Failed to fetch user data from Peloton: {e.Message}, FTP info may be missing for certain non-class workout types (Just Ride)."); + userData = await _pelotonService.GetUserDataAsync(); + } + catch (Exception e) + { + _logger.Warning(e, $"Failed to fetch user data from Peloton: {e.Message}, FTP info may be missing for certain non-class workout types (Just Ride)."); } P2GWorkout[] workouts = { }; @@ -104,31 +104,31 @@ public async Task SyncAsync(IEnumerable workoutIds, ICollect return true; }); - + var filteredWorkoutsCount = filteredWorkouts.Count(); - activity?.AddTag("workouts.filtered", filteredWorkoutsCount); - _logger.Information("Found {@NumWorkouts} workouts remaining after filtering ExcludedWorkoutTypes.", filteredWorkoutsCount); - - if (!filteredWorkouts.Any()) - { - _logger.Information("No workouts to sync. Sync complete."); - response.ConversionSuccess = true; - response.SyncSuccess = true; - return response; + activity?.AddTag("workouts.filtered", filteredWorkoutsCount); + _logger.Information("Found {@NumWorkouts} workouts remaining after filtering ExcludedWorkoutTypes.", filteredWorkoutsCount); + + if (!filteredWorkouts.Any()) + { + _logger.Information("No workouts to sync. Sync complete."); + response.ConversionSuccess = true; + response.SyncSuccess = true; + return response; } - + var convertStatuses = new List(); try - { - _logger.Information("Converting workouts..."); - var tasks = new List>(); - foreach (var workout in filteredWorkouts) - { - workout.UserData = userData; - tasks.AddRange(_converters.Select(c => c.ConvertAsync(workout))); - } - - await Task.WhenAll(tasks); + { + _logger.Information("Converting workouts..."); + var tasks = new List>(); + foreach (var workout in filteredWorkouts) + { + workout.UserData = userData; + tasks.AddRange(_converters.Select(c => c.ConvertAsync(workout))); + } + + await Task.WhenAll(tasks); convertStatuses = tasks.Select(t => t.GetAwaiter().GetResult()).ToList(); } catch (Exception e) @@ -139,54 +139,54 @@ public async Task SyncAsync(IEnumerable workoutIds, ICollect response.ConversionSuccess = false; response.Errors.Add(new ErrorResponse() { Message = $"Unexpected error. Failed to convert workouts. {e.Message} Check logs for more details." }); return response; - } - - if (!convertStatuses.Any() || convertStatuses.All(c => c.Result == ConversionResult.Skipped)) - { - _logger.Information("All converters were skipped. Ensure you have atleast one output Format configured in your settings. Converting to FIT or TCX is required prior to uploading to Garmin Connect."); - response.SyncSuccess = false; - response.ConversionSuccess = false; - response.Errors.Add(new ErrorResponse() { Message = "All converters were skipped. Ensure you have atleast one output Format configured in your settings. Converting to FIT or TCX is required prior to uploading to Garmin Connect." }); - return response; - } - - if (convertStatuses.All(c => c.Result == ConversionResult.Failed)) - { - _logger.Error("All configured converters failed to convert workouts."); - response.SyncSuccess = false; - response.ConversionSuccess = false; - response.Errors.Add(new ErrorResponse() { Message = "All configured converters failed to convert workouts. Successfully, converting to FIT or TCX is required prior to uploading to Garmin Connect. See logs for more details." }); - return response; - } - - foreach (var convertStatus in convertStatuses) - if (convertStatus.Result == ConversionResult.Failed) - response.Errors.Add(new ErrorResponse() { Message = convertStatus.ErrorMessage }); - + } + + if (!convertStatuses.Any() || convertStatuses.All(c => c.Result == ConversionResult.Skipped)) + { + _logger.Information("All converters were skipped. Ensure you have atleast one output Format configured in your settings. Converting to FIT or TCX is required prior to uploading to Garmin Connect."); + response.SyncSuccess = false; + response.ConversionSuccess = false; + response.Errors.Add(new ErrorResponse() { Message = "All converters were skipped. Ensure you have atleast one output Format configured in your settings. Converting to FIT or TCX is required prior to uploading to Garmin Connect." }); + return response; + } + + if (convertStatuses.All(c => c.Result == ConversionResult.Failed)) + { + _logger.Error("All configured converters failed to convert workouts."); + response.SyncSuccess = false; + response.ConversionSuccess = false; + response.Errors.Add(new ErrorResponse() { Message = "All configured converters failed to convert workouts. Successfully, converting to FIT or TCX is required prior to uploading to Garmin Connect. See logs for more details." }); + return response; + } + + foreach (var convertStatus in convertStatuses) + if (convertStatus.Result == ConversionResult.Failed) + response.Errors.Add(new ErrorResponse() { Message = convertStatus.ErrorMessage }); + response.ConversionSuccess = true; try { await _garminUploader.UploadToGarminAsync(); response.UploadToGarminSuccess = true; - } - catch (ArgumentException ae) - { - _logger.Error(ae, $"Failed to upload to Garmin Connect. {ae.Message}"); - + } + catch (ArgumentException ae) + { + _logger.Error(ae, $"Failed to upload to Garmin Connect. {ae.Message}"); + response.SyncSuccess = false; response.UploadToGarminSuccess = false; response.Errors.Add(new ErrorResponse() { Message = $"Failed to upload workouts to Garmin Connect. {ae.Message}" }); - return response; - } - catch (GarminAuthenticationError gae) - { - _logger.Error(gae, $"Sync failed to authenticate with Garmin. {gae.Message}"); - + return response; + } + catch (GarminAuthenticationError gae) + { + _logger.Error(gae, $"Sync failed to authenticate with Garmin. {gae.Message}"); + response.SyncSuccess = false; response.UploadToGarminSuccess = false; response.Errors.Add(new ErrorResponse() { Message = gae.Message }); - return response; + return response; } catch (Exception e) { @@ -196,7 +196,7 @@ public async Task SyncAsync(IEnumerable workoutIds, ICollect response.UploadToGarminSuccess = false; response.Errors.Add(new ErrorResponse() { Message = $"Failed to upload workouts to Garmin Connect. {e.Message}" }); return response; - } + } finally { _fileHandler.Cleanup(settings.App.DownloadDirectory); @@ -206,11 +206,11 @@ public async Task SyncAsync(IEnumerable workoutIds, ICollect response.SyncSuccess = true; return response; - } - - private IEnumerable FilterToCompletedWorkoutIds(ICollection workouts) - { - return workouts + } + + private IEnumerable FilterToCompletedWorkoutIds(ICollection workouts) + { + return workouts? .Where(w => { var shouldKeep = w.Status == "COMPLETE"; @@ -219,31 +219,31 @@ private IEnumerable FilterToCompletedWorkoutIds(ICollection wor _logger.Debug("Skipping in progress workout. {@WorkoutId} {@WorkoutStatus} {@WorkoutType} {@WorkoutTitle}", w.Id, w.Status, w.Fitness_Discipline, w.Title); return false; }) - .Select(r => r.Id); - } - - private async Task SyncWithWorkoutLoaderAsync(Func>>> loader, ICollection? exclude) - { + .Select(r => r.Id) ?? new List(); + } + + private async Task SyncWithWorkoutLoaderAsync(Func>>> loader, ICollection? exclude) + { using var activity = Tracing.Trace($"{nameof(SyncService)}.{nameof(SyncAsync)}.SyncWithWorkoutLoaderAsync"); ICollection recentWorkouts; - var syncTime = await _db.GetSyncStatusAsync(); + var syncTime = await _db.GetSyncStatusAsync(); var settings = await _settingsService.GetSettingsAsync(); - syncTime.LastSyncTime = DateTime.Now; + syncTime.LastSyncTime = DateTime.Now; try - { + { var recentWorkoutsServiceResult = await loader(); recentWorkouts = recentWorkoutsServiceResult.Result; } - catch (ArgumentException ae) - { + catch (ArgumentException ae) + { var errorMessage = $"Failed to fetch workouts from Peloton: {ae.Message}"; _logger.Error(ae, errorMessage); activity?.AddTag("exception.message", ae.Message); - activity?.AddTag("exception.stacktrace", ae.StackTrace); - + activity?.AddTag("exception.stacktrace", ae.StackTrace); + syncTime.SyncStatus = Status.UnHealthy; syncTime.LastErrorMessage = errorMessage; await _db.UpsertSyncStatusAsync(syncTime); @@ -252,7 +252,7 @@ private async Task SyncWithWorkoutLoaderAsync(Func SyncWithWorkoutLoaderAsync(Func SyncWithWorkoutLoaderAsync(Func SyncWithWorkoutLoaderAsync(Func x.Cleanup(It.IsAny()), Times.Exactly(3)); } + [Test] + public async Task SyncAsync_WhenNoWorkouts_Should_NotThrow() + { + // SETUP + var mocker = new AutoMocker(); + + var service = mocker.CreateInstance(); + var peloton = mocker.GetMock(); + var db = mocker.GetMock(); + var converter = mocker.GetMock(); + var garmin = mocker.GetMock(); + var fileHandler = mocker.GetMock(); + var settingsService = mocker.GetMock(); + + var settings = new Settings(); + settings.Format.Fit = true; + settings.App.CheckForUpdates = false; + settings.Peloton.NumWorkoutsToDownload = 5; + settingsService.Setup(s => s.GetSettingsAsync()).ReturnsAsync(settings); + + converter.Setup(c => c.ConvertAsync(It.IsAny())).ReturnsAsync(new ConvertStatus() { Result = ConversionResult.Success }); + + var syncStatus = new SyncServiceStatus(); + db.Setup(x => x.GetSyncStatusAsync()).Returns(Task.FromResult(syncStatus)); + peloton.Setup(x => x.GetWorkoutDetailsAsync(It.IsAny>())).ReturnsAsync(new P2GWorkout[] { new P2GWorkout() }); + + var result = new ServiceResult>(); + result.Result = null; + peloton.Setup(x => x.GetRecentWorkoutsAsync(It.IsAny())) + .ReturnsAsync(result) + .Verifiable(); + + // ACT + var response = await service.SyncAsync(5); + + // ASSERT + response.SyncSuccess.Should().BeTrue(); + response.PelotonDownloadSuccess.Should().BeTrue(); + response.ConversionSuccess.Should().BeTrue(); + response.UploadToGarminSuccess.Should().BeTrue(); + response.Errors.Should().BeNullOrEmpty(); + + peloton.Verify(x => x.GetRecentWorkoutsAsync(5), Times.Once); + converter.Verify(x => x.ConvertAsync(It.IsAny()), Times.Once); + garmin.Verify(x => x.UploadToGarminAsync(), Times.Once); + db.Verify(x => x.UpsertSyncStatusAsync(It.IsAny()), Times.Once); + fileHandler.Verify(x => x.Cleanup(It.IsAny()), Times.Exactly(3)); + } + [Test] public async Task SyncAsync_When_CheckForUpdates_Disabled_Should_NotCheck() { diff --git a/vNextReleaseNotes.md b/vNextReleaseNotes.md index 4c86fdfc5..1181d2642 100644 --- a/vNextReleaseNotes.md +++ b/vNextReleaseNotes.md @@ -1,23 +1,7 @@ [![](https://img.shields.io/static/v1?label=Sponsor&message=%E2%9D%A4&logo=GitHub&color=%23fe8e86)](https://github.com/sponsors/philosowaffle) Buy Me A Coffee donate button --- -## Features - -- [#502] Partial support for Peloton Gym -- [#497] Add minimal support for Rowing Bootcamp -- More exercise mappings - - [#495] Open Lateral Raise, Pike Push Up, Dolphin - - [#499] Forearm Side Plank Rotation, Straight Leg Bicycle - - [#510] Bear Crawl - - [#546] Arm Circles, Criss-Cross, High Pull, Oblique Heel Tap, Row, Single Leg Stretch, Tricep Dip -- [#532] GitHubAction now supports attaching output files to the GitHub Action as a zip file you can download - @anlesk - ## Fixes -- [#526] `Auth appeared successful but there was an error sending the service ticket to Garmin` -- [#541] Continuation of #526 but specifically for the MFA flow -- `All converters were skipped.` - confusing log message when no workouts needed to be synced - -## Housekeeping - -- [#509] Various dependency bumps +- [#552] fix ArgumentNullException in SyncService when no recent workouts found +- [#548] GithubAction - workflow_dispatch boolean inputs are not actually booleans (@philjn) \ No newline at end of file