Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix version check crash #6158

Merged
merged 5 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Rubberduck.Core/Properties/Settings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions Rubberduck.Core/Rubberduck.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,17 @@
<Version>2.0.20525</Version>
</PackageReference>
</ItemGroup>
<ItemGroup>
<Compile Update="Properties\Settings.Designer.cs">
<DesignTimeSharedInput>True</DesignTimeSharedInput>
<AutoGen>True</AutoGen>
<DependentUpon>Settings.settings</DependentUpon>
</Compile>
</ItemGroup>
<ItemGroup>
<None Update="Properties\Settings.settings">
<Generator>SettingsSingleFileGenerator</Generator>
<LastGenOutput>Settings.Designer.cs</LastGenOutput>
</None>
</ItemGroup>
</Project>
5 changes: 4 additions & 1 deletion Rubberduck.Core/Settings/GeneralSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public interface IGeneralSettings
DisplayLanguageSetting Language { get; set; }
bool CanShowSplash { get; set; }
bool CanCheckVersion { get; set; }
string ApiBaseUrl { get; set; }
bool IncludePreRelease { get; set; }
bool CompileBeforeParse { get; set; }
bool IsSmartIndenterPrompted { get; set; }
Expand Down Expand Up @@ -45,6 +46,7 @@ public DisplayLanguageSetting Language

public bool CanShowSplash { get; set; }
public bool CanCheckVersion { get; set; }
public string ApiBaseUrl { get; set; }
public bool IncludePreRelease { get; set; }
public bool CompileBeforeParse { get; set; }
public bool IsSmartIndenterPrompted { get; set; }
Expand Down Expand Up @@ -103,7 +105,8 @@ public bool Equals(GeneralSettings other)
EnableExperimentalFeatures.Count == other.EnableExperimentalFeatures.Count &&
EnableExperimentalFeatures.All(other.EnableExperimentalFeatures.Contains) &&
SetDpiUnaware == other.SetDpiUnaware &&
EnableFolderDragAndDrop == other.EnableFolderDragAndDrop;
EnableFolderDragAndDrop == other.EnableFolderDragAndDrop &&
ApiBaseUrl == other.ApiBaseUrl;
}
}
}
55 changes: 23 additions & 32 deletions Rubberduck.Core/UI/Command/VersionCheckCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,40 +53,31 @@ protected override async void OnExecute(object parameter)
Logger.Info("Executing version check...");

var tokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(5));
await _versionCheck
.GetLatestVersionAsync(settings, tokenSource.Token)
.ContinueWith(t =>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the problem; the service's return value should have been awaited here, not task-continued like this.

{
if (t.IsFaulted)
{
Logger.Warn(t.Exception);
return;
}
var latest = await _versionCheck.GetLatestVersionAsync(settings, tokenSource.Token);

if (_versionCheck.CurrentVersion < t.Result)
{
var proceed = true;
if (_versionCheck.IsDebugBuild || !settings.IncludePreRelease)
{
// if the latest version has a revision number and isn't a pre-release build,
// avoid prompting since we can't know if the build already includes the latest version.
proceed = t.Result.Revision == 0;
}
if (_versionCheck.CurrentVersion < latest)
{
var proceed = true;
if (_versionCheck.IsDebugBuild || !settings.IncludePreRelease)
{
// if the latest version has a revision number and isn't a pre-release build,
// avoid prompting since we can't know if the build already includes the latest version.
proceed = latest.Revision == 0;
}

if (proceed)
{
PromptAndBrowse(t.Result, settings.IncludePreRelease);
}
else
{
Logger.Info("Version check skips notification of an existing newer version available.");
}
}
else
{
Logger.Info("Version check completed: running current latest.");
}
});
if (proceed)
{
PromptAndBrowse(latest, settings.IncludePreRelease);
}
else
{
Logger.Info("Version check skips notification of an existing newer version available.");
}
}
else if (latest != default)
{
Logger.Info("Version check completed: running current latest.");
}
}

private void PromptAndBrowse(Version latestVersion, bool includePreRelease)
Expand Down
5 changes: 2 additions & 3 deletions Rubberduck.Core/UI/Splash2021.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System.Drawing;
using System.Windows.Forms;
using Rubberduck.VersionCheck;

namespace Rubberduck.UI
{
Expand All @@ -11,9 +10,9 @@ public Splash2021()
InitializeComponent();
}

public Splash2021(IVersionCheckService versionCheck) : this()
public Splash2021(string versionString) : this()
{
VersionLabel.Text = string.Format(Resources.RubberduckUI.Rubberduck_AboutBuild, versionCheck.VersionString);
VersionLabel.Text = string.Format(Resources.RubberduckUI.Rubberduck_AboutBuild, versionString);
VersionLabel.Parent = pictureBox1;
VersionLabel.BackColor = Color.Transparent;
}
Expand Down
60 changes: 41 additions & 19 deletions Rubberduck.Core/VersionCheck/ApiClientBase.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Newtonsoft.Json;
using Rubberduck.Settings;
using System;
using System.Net;
using System.Net.Http;
Expand All @@ -9,40 +10,69 @@

namespace Rubberduck.Client.Abstract
{
public abstract class ApiClientBase : IDisposable
public interface IHttpClientProvider
{
HttpClient GetClient();
}

public sealed class HttpClientProvider : IHttpClientProvider, IDisposable
{
private readonly Lazy<HttpClient> _client;

public HttpClientProvider(Func<HttpClient> getClient)
{
_client = new Lazy<HttpClient>(getClient);
}

public HttpClient GetClient()
{
return _client.Value;
}

public void Dispose()
{
if (_client.IsValueCreated)
{
_client.Value.Dispose();
}
}
}

public abstract class ApiClientBase
{
protected static readonly string UserAgentName = "Rubberduck";
protected static readonly string BaseUrl = "https://api.rubberduckvba.com/api/v1/";
protected static readonly string ContentTypeApplicationJson = "application/json";
protected static readonly int MaxAttempts = 3;
protected static readonly TimeSpan RetryCooldownDelay = TimeSpan.FromSeconds(1);

protected readonly Lazy<HttpClient> _client;
protected readonly IHttpClientProvider _clientProvider;
protected readonly string _baseUrl;

protected ApiClientBase()
protected ApiClientBase(IGeneralSettings settings, IHttpClientProvider clientProvider)
{
_client = new Lazy<HttpClient>(() => GetClient());
_clientProvider = clientProvider;
_baseUrl = string.IsNullOrWhiteSpace(settings.ApiBaseUrl) ? "https://api.rubberduckvba.com/api/v1/" : settings.ApiBaseUrl;
}

protected HttpClient GetClient()
{
ServicePointManager.SecurityProtocol = SecurityProtocolType.Tls12;
var client = new HttpClient();
return ConfigureClient(client);
var client = _clientProvider.GetClient();
ConfigureClient(client);
return client;
}

protected virtual HttpClient ConfigureClient(HttpClient client)
protected virtual void ConfigureClient(HttpClient client)
{
var userAgentVersion = System.Reflection.Assembly.GetExecutingAssembly().GetName().Version.ToString(3);
var userAgentHeader = new ProductInfoHeaderValue(UserAgentName, userAgentVersion);

client.DefaultRequestHeaders.UserAgent.Add(userAgentHeader);
return client;
}

protected virtual async Task<TResult> GetResponse<TResult>(string route, CancellationToken? cancellationToken = null)
{
var uri = new Uri($"{BaseUrl}{route}");
var uri = new Uri($"{_baseUrl}{route}");

var attempt = 0;
var token = cancellationToken ?? CancellationToken.None;
Expand Down Expand Up @@ -102,7 +132,7 @@ protected virtual async Task<TResult> GetResponse<TResult>(string route, Cancell

protected virtual async Task<TResult> Post<TArgs, TResult>(string route, TArgs args, CancellationToken? cancellationToken = null)
{
var uri = new Uri($"{BaseUrl}{route}");
var uri = new Uri($"{_baseUrl}{route}");
string json;
try
{
Expand Down Expand Up @@ -167,13 +197,5 @@ protected virtual async Task<TResult> Post<TArgs, TResult>(string route, TArgs a
return default;
}
}

public void Dispose()
{
if (_client.IsValueCreated)
{
_client.Value.Dispose();
}
}
}
}
14 changes: 12 additions & 2 deletions Rubberduck.Core/VersionCheck/PublicApiClient.cs
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Rubberduck.Client.Abstract;
using Rubberduck.Settings;

namespace Rubberduck.VersionCheck
{
public class PublicApiClient : ApiClientBase
public interface IPublicApiClient
{
Task<IEnumerable<Tag>> GetLatestTagsAsync(CancellationToken token);
}

public class PublicApiClient : ApiClientBase, IPublicApiClient
{
private static readonly string PublicTagsEndPoint = "public/tags";

public PublicApiClient(IGeneralSettings settings, IHttpClientProvider clientProvider)
: base(settings, clientProvider)
{
}

public async Task<IEnumerable<Tag>> GetLatestTagsAsync(CancellationToken token)
{
return await GetResponse<Tag[]>(PublicTagsEndPoint, token);
Expand Down
16 changes: 12 additions & 4 deletions Rubberduck.Core/VersionCheck/VersionCheckService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,15 @@ namespace Rubberduck.VersionCheck
public class VersionCheckService : IVersionCheckService
{
private static readonly ILogger _logger = LogManager.GetCurrentClassLogger();
private readonly IPublicApiClient _client;

/// <param name="version">That would be the version of the assembly for the <c>_Extension</c> class.</param>
public VersionCheckService(Version version)
/// <param name="client"></param>
public VersionCheckService(Version version, IPublicApiClient client)
{
CurrentVersion = version;
_client = client;

#if DEBUG
IsDebugBuild = true;
#endif
Expand All @@ -32,19 +36,23 @@ public async Task<Version> GetLatestVersionAsync(GeneralSettings settings, Cance
return _latestVersion;
}

using (var client = new PublicApiClient())
try
{
var tags = await client.GetLatestTagsAsync(token);
var tags = await _client.GetLatestTagsAsync(token);

var next = tags.Single(e => e.IsPreRelease);
var main = tags.Single(e => !e.IsPreRelease);
_logger.Info($"Main: v{main.Version.ToString(3)}; Next: v{next.Version.ToString(4)}");

_latestVersion = settings.IncludePreRelease ? next.Version : main.Version;
_logger.Info($"Check prerelease: {settings.IncludePreRelease}; latest: v{_latestVersion.ToString(4)}");
}
catch ( Exception ex )
{
_logger.Warn(ex, "Version check failed.");

return _latestVersion;
}
return _latestVersion;
}

public Version CurrentVersion { get; }
Expand Down
1 change: 1 addition & 0 deletions Rubberduck.Core/app.config
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@
<MinimumLogLevel>0</MinimumLogLevel>
<SetDpiUnaware>false</SetDpiUnaware>
<EnableExperimentalFeatures />
<ApiBaseUrl>https://api.rubberduckvba.com/api/v1/</ApiBaseUrl>
</GeneralSettings>
</value>
</setting>
Expand Down
2 changes: 1 addition & 1 deletion Rubberduck.Main/Extension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ private void InitializeAddIn()

if (_initialSettings?.CanShowSplash ?? false)
{
splash = new Splash2021(new VersionCheckService(typeof(Splash2021).Assembly.GetName().Version));
splash = new Splash2021(string.Format(RubberduckUI.Rubberduck_AboutBuild, Assembly.GetExecutingAssembly().GetName().Version.ToString(3)));
splash.Show();
splash.Refresh();
}
Expand Down
24 changes: 22 additions & 2 deletions RubberduckTests/VersionCheckTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using System;
using System.Threading;
using System.Threading.Tasks;
using System.Web;
using Moq;
using NUnit.Framework;
using Rubberduck.Interaction;
Expand All @@ -10,6 +12,24 @@

namespace RubberduckTests
{
[TestFixture]
public class VersionCheckServiceTests
{
[Test]
public async Task GetLatestVersionThrowsHttpException_IsHandled()
{
var appVersion = new Version();
var apiClient = new Mock<IPublicApiClient>();
apiClient.Setup(m => m.GetLatestTagsAsync(It.IsAny<CancellationToken>())).Throws<HttpException>();

var sut = new VersionCheckService(appVersion, apiClient.Object);

var result = await sut.GetLatestVersionAsync(new GeneralSettings(), CancellationToken.None);

Assert.IsNull(result);
}
}

[TestFixture]
public class VersionCheckTests
{
Expand All @@ -34,12 +54,12 @@ private VersionCheckCommand CreateSUT(Configuration config, Version currentVersi
mockConfig.Setup(m => m.Read()).Returns(() => config);

mockService = new Mock<IVersionCheckService>();

mockService.Setup(m => m.CurrentVersion)
.Returns(() => currentVersion);

mockService.Setup(m => m.GetLatestVersionAsync(It.IsAny<GeneralSettings>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(() => latestVersion);
.ReturnsAsync(() => latestVersion);

return new VersionCheckCommand(mockService.Object, mockPrompt.Object, mockProcess.Object, mockConfig.Object);
}
Expand Down