Skip to content

Commit

Permalink
SLCORE-1110 apply PR review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
sophio-japharidze-sonarsource committed Jan 31, 2025
1 parent d234a0d commit b0a8c3d
Show file tree
Hide file tree
Showing 20 changed files with 128 additions and 96 deletions.
15 changes: 14 additions & 1 deletion API_CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* Client can implement this method to offer user to change credentials for the connection to fix the problem
* For now notification is being sent only for 403 Forbidden HTTP response code since it's corresponds to malformed/wrong token and ignores 401 Unauthorized response code since it's a user permissions problem that has to be addressed on the server
* Also once notification sent, backend doesn't attempt to send any requests to server anymore until credentials changed
* Add `region` to `org.sonarsource.sonarlint.core.rpc.protocol.client.connection.SonarCloudConnectionParams` and `org.sonarsource.sonarlint.core.rpc.protocol.client.connection.SonarCloudConnectionSuggestionDto` to support multi-region SQC connection configuration
* Constructor without region parameter is removed

* Removed `org.sonarsource.sonarlint.core.rpc.protocol.SonarLintRpcClient#didRaiseIssue` and associated types. See `raiseIssues` and `raiseHotspots` instead.
* Removed `org.sonarsource.sonarlint.core.rpc.protocol.SonarLintRpcServer#getIssueTrackingService` and associated types. Tracking is managed by the backend.
Expand All @@ -24,7 +26,18 @@
* `getCleanCodeAttribute`
* `getImpacts`
* Use `getSeverityMode` instead.
* Add SonarCloud region parameter to `org.sonarsource.sonarlint.core.rpc.protocol.client.connection.SonarCloudConnectionParams` to support multi-region SQC connection configuration

## New features

* Add SonarCloud region parameter to
* `org.sonarsource.sonarlint.core.rpc.protocol.client.connection.SonarCloudConnectionParams`
* `org.sonarsource.sonarlint.core.rpc.protocol.backend.connection.common.TransientSonarCloudConnectionDto`
* `org.sonarsource.sonarlint.core.rpc.protocol.backend.connection.config.SonarCloudConnectionConfigurationDto`
* `org.sonarsource.sonarlint.core.rpc.protocol.backend.connection.org.FuzzySearchUserOrganizationsParams`
* `org.sonarsource.sonarlint.core.rpc.protocol.backend.connection.org.GetOrganizationParams`
* `org.sonarsource.sonarlint.core.rpc.protocol.backend.connection.org.ListUserOrganizationsParams`
* This is in order to support multi-region SQC connection configuration. Constructors without region parameter are deprecated


# 10.13

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.Properties;
import java.util.Set;
Expand Down Expand Up @@ -234,7 +235,6 @@ private static class BindingProperties {
private final String organization;
private final String serverUrl;
private final boolean isFromSharedConfiguration;
@Nullable
private final SonarCloudRegion region;

private BindingProperties(@Nullable String projectKey, @Nullable String organization, @Nullable String serverUrl, @Nullable String region, boolean isFromSharedConfiguration) {
Expand All @@ -244,9 +244,9 @@ private BindingProperties(@Nullable String projectKey, @Nullable String organiza
this.isFromSharedConfiguration = isFromSharedConfiguration;
SonarCloudRegion configuredRegion;
try {
configuredRegion = region != null ? SonarCloudRegion.valueOf(region) : SonarCloudRegion.EU;
configuredRegion = region != null ? SonarCloudRegion.valueOf(region.toUpperCase(Locale.ENGLISH)) : SonarCloudRegion.EU;
} catch (IllegalArgumentException e) {
LOG.warn("Unknown region '{}'", region);
LOG.warn("Cannot accept '{}' as a valid SonarQube Cloud region while reading shared Connected Mode settings", region);
configuredRegion = SonarCloudRegion.EU;
}
this.region = configuredRegion;
Expand All @@ -263,7 +263,8 @@ private BindingClue computeBindingClue(String filename, BindingProperties scanne
}
if (scannerProps.serverUrl != null) {
if (sonarCloudActiveEnvironment.isSonarQubeCloud(scannerProps.serverUrl)) {
return new SonarCloudBindingClue(scannerProps.projectKey, null, scannerProps.region, scannerProps.isFromSharedConfiguration);
var region = sonarCloudActiveEnvironment.getRegionOrThrow(scannerProps.serverUrl);
return new SonarCloudBindingClue(scannerProps.projectKey, null, SonarCloudRegion.valueOf(region.name()), scannerProps.isFromSharedConfiguration);
} else {
return new SonarQubeBindingClue(scannerProps.projectKey, scannerProps.serverUrl, scannerProps.isFromSharedConfiguration);
}
Expand Down Expand Up @@ -342,7 +343,7 @@ public static class SonarCloudBindingClue implements BindingClue {
this.sonarProjectKey = sonarProjectKey;
this.organization = organization;
this.isFromSharedConfiguration = isFromSharedConfiguration;
this.region = region;
this.region = region != null ? region : SonarCloudRegion.EU;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,15 @@

public class SonarCloudActiveEnvironment {

public static final URI PRODUCTION_EU_URI = URI.create("https://sonarcloud.io");
public static final URI PRODUCTION_US_URI = URI.create("https://us.sonarcloud.io");
public static final URI WS_EU_ENDPOINT_URI = URI.create("wss://events-api.sonarcloud.io/");
public static final URI WS_US_ENDPOINT_URI = URI.create("wss://events-api.us.sonarcloud.io/");

public static SonarCloudActiveEnvironment prod() {
return new SonarCloudActiveEnvironment();
}

private final Map<SonarCloudRegion, SonarQubeCloudUris> uris;

public SonarCloudActiveEnvironment() {
var euUris = new SonarQubeCloudUris(PRODUCTION_EU_URI, WS_EU_ENDPOINT_URI);
var usUris = new SonarQubeCloudUris(PRODUCTION_US_URI, WS_US_ENDPOINT_URI);
var euUris = new SonarQubeCloudUris(SonarCloudRegion.EU.getProductionUri(), SonarCloudRegion.EU.getWebSocketUri());
var usUris = new SonarQubeCloudUris(SonarCloudRegion.US.getProductionUri(), SonarCloudRegion.US.getWebSocketUri());
this.uris = Map.of(SonarCloudRegion.EU, euUris, SonarCloudRegion.US, usUris);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,25 @@
*/
package org.sonarsource.sonarlint.core;

import java.net.URI;

public enum SonarCloudRegion {
EU, US
EU("https://sonarcloud.io", "wss://events-api.sonarcloud.io/"),
US("https://us.sonarcloud.io", "wss://events-api.us.sonarcloud.io/");

private final URI productionUri;
private final URI webSocketUri;

SonarCloudRegion(String productionUri, String webSocketUri) {
this.productionUri = URI.create(productionUri);
this.webSocketUri = URI.create(webSocketUri);
}

public URI getProductionUri() {
return productionUri;
}

public URI getWebSocketUri() {
return webSocketUri;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,18 @@

import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ProtocolException;
import org.sonarsource.sonarlint.core.SonarCloudActiveEnvironment;

import static org.apache.commons.lang3.StringUtils.removeEnd;

public class RequestHandlerUtils {

private RequestHandlerUtils() {
// util
}

public static String getServerUrlForSonarCloud(ClassicHttpRequest request, SonarCloudActiveEnvironment sonarCloudActiveEnvironment) throws ProtocolException {
public static String getServerUrlForSonarCloud(ClassicHttpRequest request) throws ProtocolException {
var originUrl = request.getHeader("Origin").getValue();
// Since the 'isSonarCloud' check passed, we are sure that the region will be there
var region = sonarCloudActiveEnvironment.getRegionOrThrow(originUrl);
return sonarCloudActiveEnvironment.getUri(region).toString();
return removeEnd(originUrl, "/");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ ShowFixSuggestionQuery extractQuery(ClassicHttpRequest request, @Nullable String
boolean isSonarCloud = isSonarCloud(origin);
String serverUrl;
if (isSonarCloud) {
serverUrl = getServerUrlForSonarCloud(request, sonarCloudActiveEnvironment);
serverUrl = getServerUrlForSonarCloud(request);
} else {
serverUrl = params.get("server");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.apache.hc.core5.net.URIBuilder;
import org.sonarsource.sonarlint.core.ConnectionManager;
import org.sonarsource.sonarlint.core.SonarCloudActiveEnvironment;
import org.sonarsource.sonarlint.core.SonarCloudRegion;
import org.sonarsource.sonarlint.core.commons.progress.SonarLintCancelMonitor;
import org.sonarsource.sonarlint.core.file.FilePathTranslation;
import org.sonarsource.sonarlint.core.file.PathTranslationService;
Expand Down Expand Up @@ -132,7 +133,8 @@ private static AssistCreatingConnectionParams createAssistServerConnectionParams
var tokenName = query.getTokenName();
var tokenValue = query.getTokenValue();
return query.isSonarCloud ?
new AssistCreatingConnectionParams(new SonarCloudConnectionParams(query.getOrganizationKey(), tokenName, tokenValue))
new AssistCreatingConnectionParams(new SonarCloudConnectionParams(query.getOrganizationKey(), tokenName, tokenValue,
org.sonarsource.sonarlint.core.rpc.protocol.common.SonarCloudRegion.valueOf(query.getRegion().name())))
: new AssistCreatingConnectionParams(new SonarQubeConnectionParams(query.getServerUrl(), tokenName, tokenValue));
}

Expand Down Expand Up @@ -208,13 +210,15 @@ ShowIssueQuery extractQuery(ClassicHttpRequest request) throws ProtocolException
}
boolean isSonarCloud = isSonarCloud(request);
String serverUrl;
SonarCloudRegion region = null;
if (isSonarCloud) {
serverUrl = getServerUrlForSonarCloud(request, sonarCloudActiveEnvironment);
serverUrl = getServerUrlForSonarCloud(request);
region = sonarCloudActiveEnvironment.getRegionOrThrow(serverUrl);
} else {
serverUrl = params.get("server");
}
return new ShowIssueQuery(serverUrl, params.get("project"), params.get("issue"), params.get("branch"),
params.get("pullRequest"), params.get("tokenName"), params.get("tokenValue"), params.get("organizationKey"), isSonarCloud);
params.get("pullRequest"), params.get("tokenName"), params.get("tokenValue"), params.get("organizationKey"), region, isSonarCloud);
}

@VisibleForTesting
Expand All @@ -232,10 +236,12 @@ public static class ShowIssueQuery {
private final String tokenValue;
@Nullable
private final String organizationKey;
@Nullable
private final SonarCloudRegion region;
private final boolean isSonarCloud;

public ShowIssueQuery(@Nullable String serverUrl, String projectKey, String issueKey, @Nullable String branch, @Nullable String pullRequest,
@Nullable String tokenName, @Nullable String tokenValue, @Nullable String organizationKey, boolean isSonarCloud) {
@Nullable String tokenName, @Nullable String tokenValue, @Nullable String organizationKey, @Nullable SonarCloudRegion region, boolean isSonarCloud) {
this.serverUrl = serverUrl;
this.projectKey = projectKey;
this.issueKey = issueKey;
Expand All @@ -244,6 +250,7 @@ public ShowIssueQuery(@Nullable String serverUrl, String projectKey, String issu
this.tokenName = tokenName;
this.tokenValue = tokenValue;
this.organizationKey = organizationKey;
this.region = region != null ? region : SonarCloudRegion.EU;
this.isSonarCloud = isSonarCloud;
}

Expand Down Expand Up @@ -308,6 +315,11 @@ public String getTokenName() {
public String getTokenValue() {
return tokenValue;
}

@Nullable
public SonarCloudRegion getRegion() {
return region;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ void should_detect_sonar_scanner_for_sonarcloud_based_on_url() {
mockFindFileByNamesInScope(
List.of(buildClientFile("sonar-project.properties", "path/to/sonar-project.properties", "sonar.host.url=https://sonarcloud.io\nsonar.projectKey=" + PROJECT_KEY_1)));

when(connectionRepository.getConnectionById(SC_CONNECTION_ID_1)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudActiveEnvironment.PRODUCTION_EU_URI, SC_CONNECTION_ID_1, MY_ORG_1, SonarCloudRegion.EU, true));
when(connectionRepository.getConnectionById(SC_CONNECTION_ID_2)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudActiveEnvironment.PRODUCTION_EU_URI, SC_CONNECTION_ID_2, MY_ORG_2, SonarCloudRegion.EU, true));
when(connectionRepository.getConnectionById(SC_CONNECTION_ID_1)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudRegion.EU.getProductionUri(), SC_CONNECTION_ID_1, MY_ORG_1, SonarCloudRegion.EU, true));
when(connectionRepository.getConnectionById(SC_CONNECTION_ID_2)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudRegion.EU.getProductionUri(), SC_CONNECTION_ID_2, MY_ORG_2, SonarCloudRegion.EU, true));

var bindingClueWithConnections = underTest.collectBindingCluesWithConnections(CONFIG_SCOPE_ID, Set.of(SC_CONNECTION_ID_1, SC_CONNECTION_ID_2), new SonarLintCancelMonitor());

Expand All @@ -121,8 +121,8 @@ void should_detect_sonar_scanner_for_sonarcloud_based_on_url() {
void should_detect_sonar_scanner_for_sonarcloud_based_on_organization() {
mockFindFileByNamesInScope(List.of(buildClientFile("sonar-project.properties", "path/to/sonar-project.properties", "sonar.organization=" + MY_ORG_2)));

when(connectionRepository.getConnectionById(SC_CONNECTION_ID_1)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudActiveEnvironment.PRODUCTION_EU_URI, SC_CONNECTION_ID_1, MY_ORG_1, SonarCloudRegion.EU, true));
when(connectionRepository.getConnectionById(SC_CONNECTION_ID_2)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudActiveEnvironment.PRODUCTION_EU_URI, SC_CONNECTION_ID_2, MY_ORG_2, SonarCloudRegion.EU, true));
when(connectionRepository.getConnectionById(SC_CONNECTION_ID_1)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudRegion.EU.getProductionUri(), SC_CONNECTION_ID_1, MY_ORG_1, SonarCloudRegion.EU, true));
when(connectionRepository.getConnectionById(SC_CONNECTION_ID_2)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudRegion.EU.getProductionUri(), SC_CONNECTION_ID_2, MY_ORG_2, SonarCloudRegion.EU, true));

var bindingClueWithConnections = underTest.collectBindingCluesWithConnections(CONFIG_SCOPE_ID, Set.of(SC_CONNECTION_ID_1, SC_CONNECTION_ID_2), new SonarLintCancelMonitor());

Expand All @@ -137,7 +137,7 @@ void should_detect_sonar_scanner_for_sonarcloud_based_on_organization() {
void should_detect_autoscan_for_sonarcloud() {
mockFindFileByNamesInScope(List.of(buildClientFile(".sonarcloud.properties", "path/to/.sonarcloud.properties", "sonar.projectKey=" + PROJECT_KEY_1)));

when(connectionRepository.getConnectionById(SC_CONNECTION_ID_1)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudActiveEnvironment.PRODUCTION_EU_URI, SC_CONNECTION_ID_1, MY_ORG_1, SonarCloudRegion.EU, true));
when(connectionRepository.getConnectionById(SC_CONNECTION_ID_1)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudRegion.EU.getProductionUri(), SC_CONNECTION_ID_1, MY_ORG_1, SonarCloudRegion.EU, true));
when(connectionRepository.getConnectionById(SQ_CONNECTION_ID_1)).thenReturn(new SonarQubeConnectionConfiguration(SQ_CONNECTION_ID_1, "http://mysonarqube.org", true));

var bindingClueWithConnections = underTest.collectBindingCluesWithConnections(CONFIG_SCOPE_ID, Set.of(SC_CONNECTION_ID_1, SQ_CONNECTION_ID_1), new SonarLintCancelMonitor());
Expand All @@ -153,7 +153,7 @@ void should_detect_autoscan_for_sonarcloud() {
void should_detect_unknown_with_project_key() {
mockFindFileByNamesInScope(List.of(buildClientFile("sonar-project.properties", "path/to/sonar-project.properties", "sonar.projectKey=" + PROJECT_KEY_1)));

when(connectionRepository.getConnectionById(SC_CONNECTION_ID_1)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudActiveEnvironment.PRODUCTION_EU_URI, SC_CONNECTION_ID_1, MY_ORG_1, SonarCloudRegion.EU, true));
when(connectionRepository.getConnectionById(SC_CONNECTION_ID_1)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudRegion.EU.getProductionUri(), SC_CONNECTION_ID_1, MY_ORG_1, SonarCloudRegion.EU, true));
when(connectionRepository.getConnectionById(SQ_CONNECTION_ID_1)).thenReturn(new SonarQubeConnectionConfiguration(SQ_CONNECTION_ID_1, "http://mysonarqube.org", true));

var bindingClueWithConnections = underTest.collectBindingCluesWithConnections(CONFIG_SCOPE_ID, Set.of(SC_CONNECTION_ID_1, SQ_CONNECTION_ID_1), new SonarLintCancelMonitor());
Expand All @@ -169,7 +169,7 @@ void should_detect_unknown_with_project_key() {
void ignore_scanner_file_without_clue() {
mockFindFileByNamesInScope(List.of(buildClientFile("sonar-project.properties", "path/to/sonar-project.properties", "sonar.sources=src")));

when(connectionRepository.getConnectionById(SC_CONNECTION_ID_1)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudActiveEnvironment.PRODUCTION_EU_URI, SC_CONNECTION_ID_1, MY_ORG_1, SonarCloudRegion.EU, true));
when(connectionRepository.getConnectionById(SC_CONNECTION_ID_1)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudRegion.EU.getProductionUri(), SC_CONNECTION_ID_1, MY_ORG_1, SonarCloudRegion.EU, true));
when(connectionRepository.getConnectionById(SQ_CONNECTION_ID_1)).thenReturn(new SonarQubeConnectionConfiguration(SQ_CONNECTION_ID_1, "http://mysonarqube.org", true));

var bindingClueWithConnections = underTest.collectBindingCluesWithConnections(CONFIG_SCOPE_ID, Set.of(SC_CONNECTION_ID_1, SQ_CONNECTION_ID_1), new SonarLintCancelMonitor());
Expand All @@ -181,7 +181,7 @@ void ignore_scanner_file_without_clue() {
void ignore_scanner_file_invalid_content() {
mockFindFileByNamesInScope(List.of(buildClientFile("sonar-project.properties", "path/to/sonar-project.properties", "\\usonar.projectKey=" + PROJECT_KEY_1)));

when(connectionRepository.getConnectionById(SC_CONNECTION_ID_1)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudActiveEnvironment.PRODUCTION_EU_URI, SC_CONNECTION_ID_1, MY_ORG_1, SonarCloudRegion.EU, true));
when(connectionRepository.getConnectionById(SC_CONNECTION_ID_1)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudRegion.EU.getProductionUri(), SC_CONNECTION_ID_1, MY_ORG_1, SonarCloudRegion.EU, true));
when(connectionRepository.getConnectionById(SQ_CONNECTION_ID_1)).thenReturn(new SonarQubeConnectionConfiguration(SQ_CONNECTION_ID_1, "http://mysonarqube.org", true));

var bindingClueWithConnections = underTest.collectBindingCluesWithConnections(CONFIG_SCOPE_ID, Set.of(SC_CONNECTION_ID_1, SQ_CONNECTION_ID_1), new SonarLintCancelMonitor());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class BindingSuggestionProviderTests {
public static final String SC_1_ID = "sc1";
public static final String SQ_2_ID = "sq2";
public static final SonarQubeConnectionConfiguration SQ_1 = new SonarQubeConnectionConfiguration(SQ_1_ID, "http://mysonarqube.com", true);
public static final SonarCloudConnectionConfiguration SC_1 = new SonarCloudConnectionConfiguration(SonarCloudActiveEnvironment.PRODUCTION_EU_URI, SC_1_ID, "myorg", SonarCloudRegion.EU, true);
public static final SonarCloudConnectionConfiguration SC_1 = new SonarCloudConnectionConfiguration(SonarCloudRegion.EU.getProductionUri(), SC_1_ID, "myorg", SonarCloudRegion.EU, true);
public static final String CONFIG_SCOPE_ID_1 = "configScope1";
public static final String PROJECT_KEY_1 = "projectKey1";
public static final ServerProject SERVER_PROJECT_1 = serverProject(PROJECT_KEY_1, "Project 1");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ void it_should_calculate_connectedMode_usesSC_notDisabledNotifications_telemetry
when(configurationRepository.getAllBoundScopes()).thenReturn(Set.of(new BoundScope(configurationScopeId, connectionId, projectKey)));

var connectionConfigurationRepository = mock(ConnectionConfigurationRepository.class);
when(connectionConfigurationRepository.getConnectionById(connectionId)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudActiveEnvironment.PRODUCTION_EU_URI, connectionId, "myTestOrg", SonarCloudRegion.EU, false));
when(connectionConfigurationRepository.getConnectionById(connectionId)).thenReturn(new SonarCloudConnectionConfiguration(SonarCloudRegion.EU.getProductionUri(), connectionId, "myTestOrg", SonarCloudRegion.EU, false));
var underTest = new TelemetryServerAttributesProvider(configurationRepository, connectionConfigurationRepository, mock(RulesService.class), mock(RulesRepository.class), mock(NodeJsService.class));

var telemetryLiveAttributes = underTest.getTelemetryServerLiveAttributes();
Expand Down
Loading

0 comments on commit b0a8c3d

Please sign in to comment.