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.

Sign up for GitHub

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

Add GetConnectionIdFromUri Method to ConnectionIdHelper #5231

Merged
merged 4 commits into from
Feb 16, 2024
Merged

Add GetConnectionIdFromUri Method to ConnectionIdHelper #5231

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a8a0167
implemented
ugras-ergun-sonarsource Feb 15, 2024
b00ab0a
pr feedback
ugras-ergun-sonarsource Feb 16, 2024
529cbeb
improved tests
ugras-ergun-sonarsource Feb 16, 2024
ab10962
changed nesting to increase readbility
ugras-ergun-sonarsource Feb 16, 2024
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
20 changes: 19 additions & 1 deletion src/SLCore.UnitTests/Common/Helpers/ConnectionIdHelperTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,30 @@ public class ConnectionIdHelperTests
[DataRow("sc|", null)]
[DataRow("sc|myorganization", "https://sonarcloud.io")]
[DataRow("sc|https://someuri.abcdef/123", "https://sonarcloud.io")] // should not happen, but we ignore any value after "sc|"
public void ReturnsAsExpected(string connectionId, string expectedUri)
public void GetUriFromConnectionId_ReturnsAsExpected(string connectionId, string expectedUri)
{
var uri = new ConnectionIdHelper().GetUriFromConnectionId(connectionId);

var stringUri = uri?.ToString().Trim('/'); //trim is because uri.ToString sometimes adds / at the end. This won't be a problem in real code since we use Uri-s and not strings, but for simplicity this tests asserts string equality

stringUri.Should().BeEquivalentTo(expectedUri);
}

[TestMethod]
[DataRow(null, null, null)]
[DataRow("", "something", null)]
[DataRow("not a uri", "something", null)]
[DataRow("http://someuri.com", null, "sq|http://someuri.com")]
[DataRow("http://someuri.com", "something", "sq|http://someuri.com")]
[DataRow("https://sonarcloud.io", "something", "sc|something")]
[DataRow("https://sonarcloud.io", "", null)]
[DataRow("https://sonarcloud.io", null, null)]
public void GetConnectionIdFromUri_PassUri_ReturnsAsExpected(string uriString, string organisation, string expectedConnectionId)
{
var testSubject = new ConnectionIdHelper();

var actualConnectionId = testSubject.GetConnectionIdFromUri(uriString, organisation);

actualConnectionId.Should().Be(expectedConnectionId);
}
}
22 changes: 20 additions & 2 deletions src/SLCore/Common/Helpers/ConnectionIdHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ namespace SonarLint.VisualStudio.SLCore.Common.Helpers
internal interface IConnectionIdHelper
{
Uri GetUriFromConnectionId(string connectionId);

string GetConnectionIdFromUri(string uriString, string organisation);
}

internal class ConnectionIdHelper : IConnectionIdHelper
Expand All @@ -33,6 +35,22 @@ internal class ConnectionIdHelper : IConnectionIdHelper
private const string SonarQubePrefix = "sq|";
private static readonly Uri SonarCloudUri = new Uri("https://sonarcloud.io");
Copy link
Contributor

Choose a reason for hiding this comment

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

C#9 target-typed null can be useful here (it's not useful ultimately everywhere, this is a place where it is)

private static readonly Uri SonarCloudUri = new("https://sonarcloud.io");


public string GetConnectionIdFromUri(string uriString, string organisation)
Copy link
Contributor

Choose a reason for hiding this comment

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

BoundSonarQubeProject has Uri property, why do you use string here? The other method of this class also returns Uri and not string

{
if (!string.IsNullOrWhiteSpace(uriString) && Uri.TryCreate(uriString, UriKind.Absolute, out var uri))
{
if (uri == SonarCloudUri)
{
if (!string.IsNullOrWhiteSpace(organisation)) { return SonarCloudPrefix + organisation; }
Copy link
Contributor

Choose a reason for hiding this comment

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

newlines missing in if body

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's to reduce clutter I think the expression is simple enough to read easily.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you're treating a symptom here. This method is hard to read, I completely missed the branch where organization is null (is that even a possibility?)

}
else
{
return SonarQubePrefix + uriString;
}
}
return null;
}

public Uri GetUriFromConnectionId(string connectionId)
{
if (connectionId == null)
Expand All @@ -43,8 +61,8 @@ public Uri GetUriFromConnectionId(string connectionId)
if (connectionId.StartsWith(SonarCloudPrefix))
{
var uriString = connectionId.Substring(SonarCloudPrefix.Length);
return !string.IsNullOrWhiteSpace(uriString) ? SonarCloudUri : null;

return !string.IsNullOrWhiteSpace(uriString) ? SonarCloudUri : null;
}

if (connectionId.StartsWith(SonarQubePrefix))
Expand Down
Loading