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

SLCORE-1032 Wrong token error during sync #1157

Closed
wants to merge 2 commits into from
Closed

SLCORE-1032 Wrong token error during sync #1157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf53de4
SLCORE-1032 Wrong token error during sync
kirill-knize-sonarsource Nov 11, 2024
5f1e063
API_CHANGES.md
kirill-knize-sonarsource Nov 29, 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
2 changes: 2 additions & 0 deletions API_CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* Signature of `org.sonarsource.sonarlint.core.rpc.protocol.backend.file.DidUpdateFileSystemParams#DidUpdateFileSystemParams` was changed
* Parameter `addedOrChangedFiles` was split into `addedFiles` and `changedFiles`
* Removed parameter `branch` and `pullRequest` from `org.sonarsource.sonarlint.core.rpc.protocol.client.issue.IssueDetailsDto` as it should not be used anymore by the client.
* Add new client method `org.sonarsource.sonarlint.core.rpc.client.SonarLintRpcClientDelegate#invalidToken`
* It is called when the token is invalid and the client should notify the user about it
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I like this way of doing. SLCORE is not aware in which context the method was called. For instance, on IntelliJ, we might be calling the server from a wizard on UI thread, we are not going to display a notification during the process. Any error should be done within the scope of the wizard (so as response from the call to the server IMO).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We haven't discussed in depth the scenarios you described. This feature is more about mitigating the sync errors that lead to an inconsistent state of the app in a way that is not obvious to the user.
Situations like wrong token or network error during the wizard manipulations are not in the scope, and I think notifications to clients should be disregarded or even not sent in the first place. Unfortunately, on the CORE side, we don't track if the Web API call is performed by the client request or because of the CORE internal need. So, not sending notifications is quite tricky.
Let's discus it.


# 10.10

Expand Down
96 changes: 77 additions & 19 deletions ...rce/sonarlint/core/ServerApiProvider.java → ...rce/sonarlint/core/ConnectionManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,49 +19,71 @@
*/
package org.sonarsource.sonarlint.core;

import com.google.common.annotations.VisibleForTesting;
import java.net.URI;
import java.util.Optional;
import javax.annotation.Nullable;
import javax.inject.Named;
import javax.inject.Singleton;
import org.eclipse.lsp4j.jsonrpc.ResponseErrorException;
import org.eclipse.lsp4j.jsonrpc.messages.ResponseError;
import org.sonarsource.sonarlint.core.commons.Version;
import org.sonarsource.sonarlint.core.commons.log.SonarLintLogger;
import org.sonarsource.sonarlint.core.commons.progress.SonarLintCancelMonitor;
import org.sonarsource.sonarlint.core.http.ConnectionAwareHttpClientProvider;
import org.sonarsource.sonarlint.core.http.HttpClient;
import org.sonarsource.sonarlint.core.http.HttpClientProvider;
import org.sonarsource.sonarlint.core.repository.connection.ConnectionConfigurationRepository;
import org.sonarsource.sonarlint.core.rpc.protocol.SonarLintRpcClient;
import org.sonarsource.sonarlint.core.rpc.protocol.SonarLintRpcErrorCode;
import org.sonarsource.sonarlint.core.rpc.protocol.backend.connection.common.TransientSonarCloudConnectionDto;
import org.sonarsource.sonarlint.core.rpc.protocol.backend.connection.common.TransientSonarQubeConnectionDto;
import org.sonarsource.sonarlint.core.rpc.protocol.client.sync.InvalidTokenParams;
import org.sonarsource.sonarlint.core.rpc.protocol.common.Either;
import org.sonarsource.sonarlint.core.rpc.protocol.common.TokenDto;
import org.sonarsource.sonarlint.core.rpc.protocol.common.UsernamePasswordDto;
import org.sonarsource.sonarlint.core.serverapi.EndpointParams;
import org.sonarsource.sonarlint.core.serverapi.ServerApi;
import org.sonarsource.sonarlint.core.serverapi.ServerApiErrorHandlingWrapper;
import org.sonarsource.sonarlint.core.serverapi.ServerApiHelper;
import org.sonarsource.sonarlint.core.serverconnection.ConnectionStorage;
import org.sonarsource.sonarlint.core.serverconnection.ServerInfoSynchronizer;

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

@Named
@Singleton
public class ServerApiProvider {

public class ConnectionManager {
private static final SonarLintLogger LOG = SonarLintLogger.get();
private final ConnectionConfigurationRepository connectionRepository;
private final ConnectionAwareHttpClientProvider awareHttpClientProvider;
private final HttpClientProvider httpClientProvider;
private final URI sonarCloudUri;
private final SonarLintRpcClient client;

public ServerApiProvider(ConnectionConfigurationRepository connectionRepository, ConnectionAwareHttpClientProvider awareHttpClientProvider, HttpClientProvider httpClientProvider,
SonarCloudActiveEnvironment sonarCloudActiveEnvironment) {
public ConnectionManager(ConnectionConfigurationRepository connectionRepository, ConnectionAwareHttpClientProvider awareHttpClientProvider,
HttpClientProvider httpClientProvider, SonarCloudActiveEnvironment sonarCloudActiveEnvironment, SonarLintRpcClient client) {
this.connectionRepository = connectionRepository;
this.awareHttpClientProvider = awareHttpClientProvider;
this.httpClientProvider = httpClientProvider;
this.sonarCloudUri = sonarCloudActiveEnvironment.getUri();
this.client = client;
}

public boolean hasConnection(String connectionId) {
return getServerApi(connectionId).isPresent();
}

public Optional<ServerApiErrorHandlingWrapper> tryGetServerApiWrapper(String connectionId) {
return getServerApi(connectionId).map( serverApi -> wrapWithConnection(serverApi, connectionId));
}

public Optional<ServerApi> getServerApi(String connectionId) {
public ServerApiErrorHandlingWrapper getServerApiWrapper(String baseUrl, @Nullable String organization, String token) {
return wrap(getServerApi(baseUrl, organization, token));
}

@VisibleForTesting
Optional<ServerApi> getServerApi(String connectionId) {
var params = connectionRepository.getEndpointParams(connectionId);
if (params.isEmpty()) {
LOG.debug("Connection '{}' is gone", connectionId);
Expand All @@ -70,36 +92,28 @@ public Optional<ServerApi> getServerApi(String connectionId) {
return Optional.of(new ServerApi(params.get(), awareHttpClientProvider.getHttpClient(connectionId)));
}

public ServerApi getServerApi(String baseUrl, @Nullable String organization, String token) {
@VisibleForTesting
ServerApi getServerApi(String baseUrl, @Nullable String organization, String token) {
var params = new EndpointParams(baseUrl, removeEnd(sonarCloudUri.toString(), "/").equals(removeEnd(baseUrl, "/")), organization);
return new ServerApi(params, httpClientProvider.getHttpClientWithPreemptiveAuth(token));
}

public ServerApi getServerApiOrThrow(String connectionId) {
var params = connectionRepository.getEndpointParams(connectionId);
if (params.isEmpty()) {
var error = new ResponseError(SonarLintRpcErrorCode.CONNECTION_NOT_FOUND, "Connection '" + connectionId + "' is gone", connectionId);
throw new ResponseErrorException(error);
}
return new ServerApi(params.get(), awareHttpClientProvider.getHttpClient(connectionId));
}

/**
* Used to do SonarCloud requests before knowing the organization
*/
public ServerApi getForSonarCloudNoOrg(Either<TokenDto, UsernamePasswordDto> credentials) {
public ServerApiErrorHandlingWrapper getForSonarCloudNoOrg(Either<TokenDto, UsernamePasswordDto> credentials) {
var endpointParams = new EndpointParams(sonarCloudUri.toString(), true, null);
var httpClient = getClientFor(credentials);
return new ServerApi(new ServerApiHelper(endpointParams, httpClient));
return wrap(new ServerApi(new ServerApiHelper(endpointParams, httpClient)));
}

public ServerApi getForTransientConnection(Either<TransientSonarQubeConnectionDto, TransientSonarCloudConnectionDto> transientConnection) {
public ServerApiErrorHandlingWrapper getForTransientConnection(Either<TransientSonarQubeConnectionDto, TransientSonarCloudConnectionDto> transientConnection) {
var endpointParams = transientConnection.map(
sq -> new EndpointParams(sq.getServerUrl(), false, null),
sc -> new EndpointParams(sonarCloudUri.toString(), true, sc.getOrganization()));
var httpClient = getClientFor(transientConnection
.map(TransientSonarQubeConnectionDto::getCredentials, TransientSonarCloudConnectionDto::getCredentials));
return new ServerApi(new ServerApiHelper(endpointParams, httpClient));
return wrap(new ServerApi(new ServerApiHelper(endpointParams, httpClient)));
}

private HttpClient getClientFor(Either<TokenDto, UsernamePasswordDto> credentials) {
Expand All @@ -108,4 +122,48 @@ private HttpClient getClientFor(Either<TokenDto, UsernamePasswordDto> credential
userPass -> httpClientProvider.getHttpClientWithPreemptiveAuth(userPass.getUsername(), userPass.getPassword()));
}

private void notifyClientAboutWrongToken(@Nullable String connectionId) {
client.invalidToken(new InvalidTokenParams(connectionId));
}

private ServerApiErrorHandlingWrapper wrap(ServerApi serverApi) {
return new ServerApiErrorHandlingWrapper(serverApi, () -> notifyClientAboutWrongToken(null));
}

private ServerApiErrorHandlingWrapper wrapWithConnection(ServerApi serverApi, String connectionId) {
return new ServerApiErrorHandlingWrapper(serverApi, () -> notifyClientAboutWrongToken(connectionId));
}

public boolean isSonarCloud(String connectionId) {
return getServerApiWrapperOrThrow(connectionId).isSonarCloud();
}

public Version getSonarServerVersion(String connectionId, ConnectionStorage storage, SonarLintCancelMonitor cancelMonitor) {
var serverApiWrapper = getServerApiWrapperOrThrow(connectionId);
var serverInfoSynchronizer = new ServerInfoSynchronizer(storage);
return serverInfoSynchronizer.readOrSynchronizeServerInfo(serverApiWrapper, cancelMonitor).getVersion();
}

public void throwIfNoConnection(String connectionId) {
if (getServerApi(connectionId).isEmpty()) {
throw unknownConnection(connectionId);
}
}

public ServerApiErrorHandlingWrapper getServerApiWrapperOrThrow(String connectionId) {
return wrapWithConnection(getServerApiOrThrow(connectionId), connectionId);
}

private static ResponseErrorException unknownConnection(String connectionId) {
var error = new ResponseError(SonarLintRpcErrorCode.CONNECTION_NOT_FOUND, "Connection with ID '" + connectionId + "' does not exist", connectionId);
return new ResponseErrorException(error);
}

private ServerApi getServerApiOrThrow(String connectionId) {
var params = connectionRepository.getEndpointParams(connectionId);
if (params.isEmpty()) {
throw unknownConnection(connectionId);
}
return new ServerApi(params.get(), awareHttpClientProvider.getHttpClient(connectionId));
}
}
31 changes: 15 additions & 16 deletions backend/core/src/main/java/org/sonarsource/sonarlint/core/ConnectionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,23 +63,23 @@ public class ConnectionService {
private final ApplicationEventPublisher applicationEventPublisher;
private final ConnectionConfigurationRepository repository;
private final URI sonarCloudUri;
private final ServerApiProvider serverApiProvider;
private final ConnectionManager connectionManager;
private final TokenGeneratorHelper tokenGeneratorHelper;

@Inject
public ConnectionService(ApplicationEventPublisher applicationEventPublisher, ConnectionConfigurationRepository repository, InitializeParams params,
SonarCloudActiveEnvironment sonarCloudActiveEnvironment, TokenGeneratorHelper tokenGeneratorHelper, ServerApiProvider serverApiProvider) {
this(applicationEventPublisher, repository, params.getSonarQubeConnections(), params.getSonarCloudConnections(), sonarCloudActiveEnvironment, serverApiProvider,
SonarCloudActiveEnvironment sonarCloudActiveEnvironment, TokenGeneratorHelper tokenGeneratorHelper, ConnectionManager connectionManager) {
this(applicationEventPublisher, repository, params.getSonarQubeConnections(), params.getSonarCloudConnections(), sonarCloudActiveEnvironment, connectionManager,
tokenGeneratorHelper);
}

ConnectionService(ApplicationEventPublisher applicationEventPublisher, ConnectionConfigurationRepository repository,
@Nullable List<SonarQubeConnectionConfigurationDto> initSonarQubeConnections, @Nullable List<SonarCloudConnectionConfigurationDto> initSonarCloudConnections,
SonarCloudActiveEnvironment sonarCloudActiveEnvironment, ServerApiProvider serverApiProvider, TokenGeneratorHelper tokenGeneratorHelper) {
SonarCloudActiveEnvironment sonarCloudActiveEnvironment, ConnectionManager connectionManager, TokenGeneratorHelper tokenGeneratorHelper) {
this.applicationEventPublisher = applicationEventPublisher;
this.repository = repository;
this.sonarCloudUri = sonarCloudActiveEnvironment.getUri();
this.serverApiProvider = serverApiProvider;
this.connectionManager = connectionManager;
this.tokenGeneratorHelper = tokenGeneratorHelper;
if (initSonarQubeConnections != null) {
initSonarQubeConnections.forEach(c -> repository.addOrReplace(adapt(c)));
Expand Down Expand Up @@ -157,15 +157,15 @@ private void updateConnection(AbstractConnectionConfiguration connectionConfigur

public ValidateConnectionResponse validateConnection(Either<TransientSonarQubeConnectionDto, TransientSonarCloudConnectionDto> transientConnection,
SonarLintCancelMonitor cancelMonitor) {
var serverApi = serverApiProvider.getForTransientConnection(transientConnection);
var serverChecker = new ServerVersionAndStatusChecker(serverApi);
var serverApiWrapper = connectionManager.getForTransientConnection(transientConnection);
var serverChecker = new ServerVersionAndStatusChecker(serverApiWrapper);
try {
serverChecker.checkVersionAndStatus(cancelMonitor);
var validateCredentials = serverApi.authentication().validate(cancelMonitor);
var validateCredentials = serverApiWrapper.validateAuthentication(cancelMonitor);
if (validateCredentials.success() && transientConnection.isRight()) {
var organizationKey = transientConnection.getRight().getOrganization();
if (organizationKey != null) {
var organization = serverApi.organization().getOrganization(organizationKey, cancelMonitor);
var organization = serverApiWrapper.getOrganization(organizationKey, cancelMonitor);
if (organization.isEmpty()) {
return new ValidateConnectionResponse(false, "No organizations found for key: " + organizationKey);
}
Expand All @@ -179,28 +179,27 @@ public ValidateConnectionResponse validateConnection(Either<TransientSonarQubeCo

public boolean checkSmartNotificationsSupported(Either<TransientSonarQubeConnectionDto, TransientSonarCloudConnectionDto> transientConnection,
SonarLintCancelMonitor cancelMonitor) {
var serverApi = serverApiProvider.getForTransientConnection(transientConnection);
var developersApi = serverApi.developers();
return developersApi.isSupported(cancelMonitor);
var serverApi = connectionManager.getForTransientConnection(transientConnection);
return serverApi.isSmartNotificationsSupported(cancelMonitor);
}

public HelpGenerateUserTokenResponse helpGenerateUserToken(String serverUrl, SonarLintCancelMonitor cancelMonitor) {
return tokenGeneratorHelper.helpGenerateUserToken(serverUrl, cancelMonitor);
}

public List<SonarProjectDto> getAllProjects(Either<TransientSonarQubeConnectionDto, TransientSonarCloudConnectionDto> transientConnection, SonarLintCancelMonitor cancelMonitor) {
var serverApi = serverApiProvider.getForTransientConnection(transientConnection);
return serverApi.component().getAllProjects(cancelMonitor)
var serverApiWrapper = connectionManager.getForTransientConnection(transientConnection);
return serverApiWrapper.getAllProjects(cancelMonitor)
.stream().map(serverProject -> new SonarProjectDto(serverProject.getKey(), serverProject.getName()))
.collect(Collectors.toList());
}

public Map<String, String> getProjectNamesByKey(Either<TransientSonarQubeConnectionDto, TransientSonarCloudConnectionDto> transientConnection,
List<String> projectKeys, SonarLintCancelMonitor cancelMonitor) {
var serverApi = serverApiProvider.getForTransientConnection(transientConnection);
var serverApiWrapper = connectionManager.getForTransientConnection(transientConnection);
var projectNamesByKey = new HashMap<String, String>();
projectKeys.forEach(key -> {
var projectName = serverApi.component().getProject(key, cancelMonitor).map(ServerProject::getName).orElse(null);
var projectName = serverApiWrapper.getProject(key, cancelMonitor).map(ServerProject::getName).orElse(null);
projectNamesByKey.put(key, projectName);
});
return projectNamesByKey;
Expand Down
14 changes: 7 additions & 7 deletions backend/core/src/main/java/org/sonarsource/sonarlint/core/OrganizationsCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@
public class OrganizationsCache {

private static final SonarLintLogger LOG = SonarLintLogger.get();
private final ServerApiProvider serverApiProvider;
private final ConnectionManager connectionManager;

private final Cache<Either<TokenDto, UsernamePasswordDto>, TextSearchIndex<OrganizationDto>> textSearchIndexCacheByCredentials = CacheBuilder.newBuilder()
.expireAfterWrite(5, TimeUnit.MINUTES)
.build();

public OrganizationsCache(ServerApiProvider serverApiProvider) {
this.serverApiProvider = serverApiProvider;
public OrganizationsCache(ConnectionManager connectionManager) {
this.connectionManager = connectionManager;
}

public List<OrganizationDto> fuzzySearchOrganizations(Either<TokenDto, UsernamePasswordDto> credentials, String searchText, SonarLintCancelMonitor cancelMonitor) {
Expand All @@ -74,8 +74,8 @@ public TextSearchIndex<OrganizationDto> getTextSearchIndex(Either<TokenDto, User
LOG.debug("Load user organizations...");
List<OrganizationDto> orgs;
try {
var serverApi = serverApiProvider.getForSonarCloudNoOrg(credentials);
var serverOrganizations = serverApi.organization().listUserOrganizations(cancelMonitor);
var serverApiWrapper = connectionManager.getForSonarCloudNoOrg(credentials);
var serverOrganizations = serverApiWrapper.listUserOrganizations(cancelMonitor);
orgs = serverOrganizations.stream().map(o -> new OrganizationDto(o.getKey(), o.getName(), o.getDescription())).collect(Collectors.toList());
} catch (Exception e) {
LOG.error("Error while querying SonarCloud organizations", e);
Expand Down Expand Up @@ -103,8 +103,8 @@ public List<OrganizationDto> listUserOrganizations(Either<TokenDto, UsernamePass

@CheckForNull
public OrganizationDto getOrganization(Either<TokenDto, UsernamePasswordDto> credentials, String organizationKey, SonarLintCancelMonitor cancelMonitor) {
var helper = serverApiProvider.getForSonarCloudNoOrg(credentials);
var serverOrganization = helper.organization().getOrganization(organizationKey, cancelMonitor);
var serverApiWrapper = connectionManager.getForSonarCloudNoOrg(credentials);
var serverOrganization = serverApiWrapper.getOrganization(organizationKey, cancelMonitor);
return serverOrganization.map(o -> new OrganizationDto(o.getKey(), o.getName(), o.getDescription())).orElse(null);
}

Expand Down
10 changes: 5 additions & 5 deletions backend/core/src/main/java/org/sonarsource/sonarlint/core/SonarProjectsCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
public class SonarProjectsCache {

private static final SonarLintLogger LOG = SonarLintLogger.get();
private final ServerApiProvider serverApiProvider;
private final ConnectionManager connectionManager;

private final Cache<String, TextSearchIndex<ServerProject>> textSearchIndexCacheByConnectionId = CacheBuilder.newBuilder()
.expireAfterWrite(1, TimeUnit.HOURS)
Expand Down Expand Up @@ -94,8 +94,8 @@ public int hashCode() {
}
}

public SonarProjectsCache(ServerApiProvider serverApiProvider) {
this.serverApiProvider = serverApiProvider;
public SonarProjectsCache(ConnectionManager connectionManager) {
this.connectionManager = connectionManager;
}

@EventListener
Expand All @@ -120,7 +120,7 @@ public Optional<ServerProject> getSonarProject(String connectionId, String sonar
return singleProjectsCache.get(new SonarProjectKey(connectionId, sonarProjectKey), () -> {
LOG.debug("Query project '{}' on connection '{}'...", sonarProjectKey, connectionId);
try {
return serverApiProvider.getServerApi(connectionId).flatMap(s -> s.component().getProject(sonarProjectKey, cancelMonitor));
return connectionManager.tryGetServerApiWrapper(connectionId).flatMap(s -> s.getProject(sonarProjectKey, cancelMonitor));
} catch (Exception e) {
LOG.error("Error while querying project '{}' from connection '{}'", sonarProjectKey, connectionId, e);
return Optional.empty();
Expand All @@ -137,7 +137,7 @@ public TextSearchIndex<ServerProject> getTextSearchIndex(String connectionId, So
LOG.debug("Load projects from connection '{}'...", connectionId);
List<ServerProject> projects;
try {
projects = serverApiProvider.getServerApi(connectionId).map(s -> s.component().getAllProjects(cancelMonitor)).orElse(List.of());
projects = connectionManager.tryGetServerApiWrapper(connectionId).map(s -> s.getAllProjects(cancelMonitor)).orElse(List.of());
} catch (Exception e) {
LOG.error("Error while querying projects from connection '{}'", connectionId, e);
return new TextSearchIndex<>();
Expand Down
Loading