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

Jump to bottom

Re-use location info among all REST clients #204

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Re-use location info among all REST clients #204

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
369ff86
Re-use location info among all REST clients
AlexRukhlin Jun 4, 2017
acc0f04
Some additional changes from home?
AlexRukhlin Jun 6, 2017
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 ...om/microsoft/tfs/client/common/ui/teambuild/teamexplorer/favorites/BuildFavoriteItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ private static BuildDefinition getBuildDefinitionFromFavorite(

final TFSTeamProjectCollection connection = server.getConnection();
final BuildHttpClient buildClient =
new BuildHttpClient(new TeeClientHandler(connection.getHTTPClient()), connection.getBaseURI());
new BuildHttpClient(new TeeClientHandler(connection), connection.getBaseURI());

final DefinitionReference definition =
buildClient.getDefinition(oldDefinition.getProject().getId(), oldDefinition.getId(), null, null);
Expand Down
2 changes: 1 addition & 1 deletion .../client/common/ui/teambuild/teamexplorer/sections/TeamExplorerBuildsFavoritesSection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ protected void loadDefinitions(final TeamExplorerContext context) {

final TFSTeamProjectCollection connection = context.getServer().getConnection();
final BuildHttpClient buildClient =
new BuildHttpClient(new TeeClientHandler(connection.getHTTPClient()), connection.getBaseURI());
new BuildHttpClient(new TeeClientHandler(connection), connection.getBaseURI());

final UUID projectId = UUID.fromString(context.getCurrentProjectInfo().getGUID());
final List<BuildDefinitionReference> rawDefinitions = buildClient.getDefinitions(projectId);
Expand Down
4 changes: 2 additions & 2 deletions ...t/common/ui/teambuild/teamexplorer/sections/TeamExplorerBuildsVNextDefinitionSection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ private void createNewDefinition() {
template = null;
} else {
final BuildHttpClient buildClient =
new BuildHttpClient(new TeeClientHandler(connection.getHTTPClient()), connection.getBaseURI());
new BuildHttpClient(new TeeClientHandler(connection), connection.getBaseURI());

final List<BuildDefinitionTemplate> templates = buildClient.getTemplates(projectName);

Expand Down Expand Up @@ -201,7 +201,7 @@ private void getBuildDefinitions(final TeamExplorerContext context) {

final TFSTeamProjectCollection connection = context.getServer().getConnection();
final BuildHttpClient buildClient =
new BuildHttpClient(new TeeClientHandler(connection.getHTTPClient()), connection.getBaseURI());
new BuildHttpClient(new TeeClientHandler(connection), connection.getBaseURI());

final UUID projectId = UUID.fromString(context.getCurrentProjectInfo().getGUID());
final List<BuildDefinitionReference> rawDefinitions = buildClient.getDefinitions(projectId);
Expand Down
2 changes: 1 addition & 1 deletion ...on.ui/src/com/microsoft/tfs/client/common/ui/wizard/common/WizardServerSelectionPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ private List<Account> getUserAccounts(
*/
final TFSConnection vstsConnection =
new TFSTeamProjectCollection(URIUtils.VSTS_ROOT_URL, azureAccessToken, new UIClientConnectionAdvisor());
final TeeClientHandler clientHandler = new TeeClientHandler(vstsConnection.getHTTPClient());
final TeeClientHandler clientHandler = new TeeClientHandler(vstsConnection);

final ProfileHttpClient profileClient = new ProfileHttpClient(clientHandler, URIUtils.VSTS_ROOT_URL);

Expand Down
48 changes: 30 additions & 18 deletions source/com.microsoft.tfs.core/rest_core/com/microsoft/alm/client/TeeClientHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.microsoft.alm.visualstudio.services.webapi.ApiResourceLocationCollection;
import com.microsoft.alm.visualstudio.services.webapi.ApiResourceVersion;
import com.microsoft.tfs.core.Messages;
import com.microsoft.tfs.core.TFSConnection;
import com.microsoft.tfs.core.httpclient.Header;
import com.microsoft.tfs.core.httpclient.HttpClient;
import com.microsoft.tfs.core.httpclient.HttpException;
Expand All @@ -51,11 +52,18 @@ public class TeeClientHandler extends VssRestClientHandlerBase implements VssRes
private final static String MEDIA_TYPE_PARAMETERS_SEPARATOR = ";"; //$NON-NLS-1$

private final HttpClient httpClient;
private final TFSConnection connection;

public TeeClientHandler(final HttpClient httpClient) {
this.connection = null;
this.httpClient = httpClient;
}

public TeeClientHandler(final TFSConnection connection) {
this.connection = connection;
this.httpClient = connection.getHTTPClient();
}

@Override
public boolean checkConnection() {
log.debug("Checking REST client connection"); //$NON-NLS-1$
Expand Down Expand Up @@ -90,25 +98,29 @@ public boolean checkConnection() {

@Override
public ApiResourceLocationCollection loadLocations() {
final URI optionsTarget = URIUtils.resolve(getBaseUrl(), OPTIONS_RELATIVE_PATH);
final HttpMethodBase request = createHttpMethod(HttpMethod.OPTIONS, optionsTarget);
request.setFollowRedirects(true);

try {
request.setRequestHeader(VssHttpHeaders.ACCEPT, VssMediaTypes.APPLICATION_JSON_TYPE);
final int statusCode = httpClient.executeMethod(request);

if (HttpStatus.isSuccessFamily(statusCode)) {
return JsonHelper.deserializeResponce(request, ApiResourceLocationCollection.class);
} else {
throw new HttpException(HttpStatus.getStatusText(statusCode));
if (connection != null && connection.getBaseURI().equals(getBaseUrl())) {
return connection.getServerApiLocations();
} else {
final URI optionsTarget = URIUtils.resolve(getBaseUrl(), OPTIONS_RELATIVE_PATH);
final HttpMethodBase request = createHttpMethod(HttpMethod.OPTIONS, optionsTarget);
request.setFollowRedirects(true);

try {
request.setRequestHeader(VssHttpHeaders.ACCEPT, VssMediaTypes.APPLICATION_JSON_TYPE);
final int statusCode = httpClient.executeMethod(request);

if (HttpStatus.isSuccessFamily(statusCode)) {
return JsonHelper.deserializeResponce(request, ApiResourceLocationCollection.class);
} else {
throw new HttpException(HttpStatus.getStatusText(statusCode));
}
} catch (final Exception e) {
log.error(e.getMessage(), e);
setLastException(e);
return null;
} finally {
request.releaseConnection();
}
} catch (final Exception e) {
log.error(e.getMessage(), e);
setLastException(e);
return null;
} finally {
request.releaseConnection();
}
}

Expand Down
23 changes: 18 additions & 5 deletions source/com.microsoft.tfs.core/src/com/microsoft/tfs/core/TFSConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,8 @@ public abstract class TFSConnection implements Closable {

private Version serverApiVersion = null;
private final Object serverApiVersionLock = new Object();
private ApiResourceLocationCollection serverApiLocations = null;
private final Object serverApiLocationsLock = new Object();

/**
* Creates a {@link TFSConnection}. Both a {@link URI} and a
Expand Down Expand Up @@ -1351,11 +1353,7 @@ public Version getServerApiVersion() {

synchronized (serverApiVersionLock) {
if (serverApiVersion == null) {

final TeeClientHandler clientHandler = new TeeClientHandler(getHTTPClient());
clientHandler.init(true, null, getBaseURI());

ApiResourceLocationCollection locations = clientHandler.getLocations();
ApiResourceLocationCollection locations = getServerApiLocations();
Copy link

Choose a reason for hiding this comment

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

There's at least two locks taken when inside this line. Do we have a convention (e.g. should we add a comment or something) to make sure that no future modifications take these two locks in a different order? (serverApiVersionLock and serverApiLocationsLock)

(Alternatively, I don't see a problem with locking on the same [serverApiVersionLock] when setting serverApiLocations.)


serverApiVersion = new Version(0, 0);
for (ApiResourceLocation location : locations.getLocations()) {
Expand All @@ -1371,6 +1369,21 @@ public Version getServerApiVersion() {
return serverApiVersion;
}

public ApiResourceLocationCollection getServerApiLocations() {

synchronized (serverApiLocationsLock) {
if (serverApiLocations == null) {

final TeeClientHandler clientHandler = new TeeClientHandler(getHTTPClient());
clientHandler.init(false, null, getBaseURI());

serverApiLocations = clientHandler.loadLocations();
}
}

return serverApiLocations;
}

/**
* Obtains the {@link PersistenceStoreProvider} that determines where cache
* and configuration data is stored.
Expand Down