Skip to content

Commit

Permalink
CHE-6114: Fix problems with project explorer performance in big proje…
Browse files Browse the repository at this point in the history
…cts (#6175)

1. Add ability to get Git status with filter.
2. Reworked Injecting VCS status info in Project service to make Status request only once instead of requesting each time in the loop.
3. Removed `format` parameter from get status method in Git Service, because it is redundant and do not affects to anything, since we use Jgit as a Git engine.
  • Loading branch information
vinokurig committed Sep 12, 2017
1 parent c7ed1d8 commit fc35ba2
Show file tree
Hide file tree
Showing 27 changed files with 280 additions and 130 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.eclipse.che.api.git.shared.Revision;
import org.eclipse.che.api.git.shared.ShowFileContentResponse;
import org.eclipse.che.api.git.shared.Status;
import org.eclipse.che.api.git.shared.StatusFormat;
import org.eclipse.che.api.promises.client.Promise;
import org.eclipse.che.commons.annotation.Nullable;
import org.eclipse.che.ide.resource.Path;
Expand Down Expand Up @@ -345,17 +344,20 @@ Promise<LogResponse> log(
* </pre>
*
* @param project project (root of GIT repository)
* @param format to show in short format or not
*/
Promise<String> statusText(Path project, StatusFormat format);
Promise<String> statusText(Path project);

/**
* Returns the current working tree status.
* Returns the current status.
*
* @param project the project.
* @param filter list of paths to filter the status. Status result will include only files witch
* paths are contained in the filter list, or are children of the folder paths that are
* mentioned in the filter list. Unfiltered status of working tree will be returned, if the
* filter list is empty
* @return the promise which either resolves working tree status or rejects with an error
*/
Promise<Status> getStatus(Path project);
Promise<Status> getStatus(Path project, List<String> filter);

/**
* Remove the git repository from given path.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import static java.util.Arrays.stream;
import static java.util.stream.Collectors.toList;
import static org.eclipse.che.api.git.shared.AddRequest.DEFAULT_PATTERN;
import static org.eclipse.che.api.git.shared.StatusFormat.PORCELAIN;
import static org.eclipse.che.ide.MimeType.APPLICATION_JSON;
import static org.eclipse.che.ide.MimeType.TEXT_PLAIN;
import static org.eclipse.che.ide.rest.HTTPHeader.ACCEPT;
Expand Down Expand Up @@ -45,7 +44,6 @@
import org.eclipse.che.api.git.shared.Revision;
import org.eclipse.che.api.git.shared.ShowFileContentResponse;
import org.eclipse.che.api.git.shared.Status;
import org.eclipse.che.api.git.shared.StatusFormat;
import org.eclipse.che.api.promises.client.Promise;
import org.eclipse.che.ide.MimeType;
import org.eclipse.che.ide.api.app.AppContext;
Expand Down Expand Up @@ -131,8 +129,8 @@ public Promise<Void> clone(Path project, String remoteUri, String remoteName) {
}

@Override
public Promise<String> statusText(Path project, StatusFormat format) {
String params = "?projectPath=" + project + "&format=" + format;
public Promise<String> statusText(Path project) {
String params = "?projectPath=" + project;
String url = getWsAgentBaseUrl() + STATUS + params;

return asyncRequestFactory
Expand Down Expand Up @@ -249,8 +247,15 @@ public Promise<List<Branch>> branchList(Path project, BranchListMode listMode) {
}

@Override
public Promise<Status> getStatus(Path project) {
String params = "?projectPath=" + project + "&format=" + PORCELAIN;
public Promise<Status> getStatus(Path project, List<String> filter) {
StringBuilder params = new StringBuilder("?projectPath=" + project);
if (filter != null) {
for (String path : filter) {
if (!path.isEmpty()) {
params.append("&filter=").append(path);
}
}
}
String url = getWsAgentBaseUrl() + STATUS + params;
return asyncRequestFactory
.createGetRequest(url)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@

import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
import org.eclipse.che.ide.FontAwesome;
import org.eclipse.che.ide.api.action.ActionEvent;
import org.eclipse.che.ide.api.app.AppContext;
Expand Down Expand Up @@ -72,8 +74,12 @@ public void actionPerformed(ActionEvent e) {
final GitOutputConsole console =
gitOutputConsoleFactory.create(constant.addToIndexCommandName());
consolesPanelPresenter.addCommandOutput(appContext.getDevMachine().getId(), console);
List<String> selected =
Arrays.stream(appContext.getResources())
.map(path -> path.getLocation().removeFirstSegments(1).toString())
.collect(Collectors.toList());
service
.getStatus(appContext.getRootProject().getLocation())
.getStatus(appContext.getRootProject().getLocation(), selected)
.then(
status -> {
if (containsInSelected(status.getUntracked())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import static com.google.common.collect.Iterables.getFirst;
import static java.util.Arrays.stream;
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static java.util.stream.Collectors.joining;
import static org.eclipse.che.api.git.shared.BranchListMode.LIST_REMOTE;
Expand Down Expand Up @@ -127,7 +128,7 @@ public void showDialog(Project project) {
if (getErrorCode(error.getCause())
== ErrorCodes.INIT_COMMIT_WAS_NOT_PERFORMED) {
service
.getStatus(project.getLocation())
.getStatus(project.getLocation(), emptyList())
.then(
status -> {
view.setEnableAmendCheckBox(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
import org.eclipse.che.api.git.shared.IndexFile;
import org.eclipse.che.ide.api.app.AppContext;
import org.eclipse.che.ide.api.dialogs.DialogFactory;
Expand Down Expand Up @@ -85,8 +88,13 @@ public ResetFilesPresenter(
public void showDialog(Project project) {
this.project = project;

List<String> selected =
Arrays.stream(appContext.getResources())
.map(path -> path.getLocation().removeFirstSegments(1).toString())
.collect(Collectors.toList());

service
.getStatus(project.getLocation())
.getStatus(project.getLocation(), selected)
.then(
status -> {
if (status.isClean()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
*/
package org.eclipse.che.ide.ext.git.client.status;

import static org.eclipse.che.api.git.shared.StatusFormat.LONG;
import static org.eclipse.che.ide.api.notification.StatusNotification.DisplayMode.FLOAT_MODE;
import static org.eclipse.che.ide.api.notification.StatusNotification.Status.FAIL;

Expand Down Expand Up @@ -65,7 +64,7 @@ public StatusCommandPresenter(
/** Show status. */
public void showStatus(Project project) {
service
.statusText(project.getLocation(), LONG)
.statusText(project.getLocation())
.then(
status -> {
printGitStatus(status);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/
package org.eclipse.che.ide.ext.git.client.commit;

import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static org.eclipse.che.ide.api.notification.StatusNotification.DisplayMode.FLOAT_MODE;
import static org.eclipse.che.ide.api.notification.StatusNotification.Status.FAIL;
Expand Down Expand Up @@ -122,7 +123,7 @@ public void disarm() {
.thenReturn(pushPromise);
when(service.log(any(Path.class), eq(null), anyInt(), anyInt(), anyBoolean()))
.thenReturn(logPromise);
when(service.getStatus(any(Path.class))).thenReturn(statusPromise);
when(service.getStatus(any(Path.class), eq(emptyList()))).thenReturn(statusPromise);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/
package org.eclipse.che.ide.ext.git.client.reset.files;

import static java.util.Collections.emptyList;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyObject;
Expand Down Expand Up @@ -66,7 +67,7 @@ public void disarm() {
when(indexFile.withIndexed(anyBoolean())).thenReturn(indexFile);
when(indexFile.withPath(anyString())).thenReturn(indexFile);
when(indexFile.getPath()).thenReturn("foo");
when(service.getStatus(any(Path.class))).thenReturn(statusPromise);
when(service.getStatus(any(Path.class), eq(emptyList()))).thenReturn(statusPromise);
when(statusPromise.then(any(Operation.class))).thenReturn(statusPromise);
when(statusPromise.catchError(any(Operation.class))).thenReturn(statusPromise);
when(appContext.getResources()).thenReturn(new Resource[] {});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import org.eclipse.che.api.git.shared.StatusFormat;
import org.eclipse.che.api.promises.client.Operation;
import org.eclipse.che.ide.api.notification.StatusNotification;
import org.eclipse.che.ide.ext.git.client.BaseTest;
Expand Down Expand Up @@ -54,7 +53,7 @@ public void disarm() {
constant,
notificationManager);

when(service.statusText(any(Path.class), any(StatusFormat.class))).thenReturn(stringPromise);
when(service.statusText(any(Path.class))).thenReturn(stringPromise);
when(stringPromise.then(any(Operation.class))).thenReturn(stringPromise);
when(stringPromise.catchError(any(Operation.class))).thenReturn(stringPromise);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
*/
package org.eclipse.che.plugin.pullrequest.client.vcs;

import static java.util.Collections.emptyList;

import com.google.gwt.user.client.rpc.AsyncCallback;
import com.google.inject.Inject;
import com.google.inject.Singleton;
Expand Down Expand Up @@ -137,15 +139,15 @@ public void deleteRemote(
@Override
public Promise<String> getBranchName(ProjectConfig project) {
return service
.getStatus(Path.valueOf(project.getPath()))
.getStatus(Path.valueOf(project.getPath()), emptyList())
.then((Function<Status, String>) status -> status.getBranchName());
}

@Override
public void hasUncommittedChanges(
@NotNull final ProjectConfig project, @NotNull final AsyncCallback<Boolean> callback) {
service
.getStatus(Path.valueOf(project.getPath()))
.getStatus(Path.valueOf(project.getPath()), emptyList())
.then(
status -> {
callback.onSuccess(!status.isClean());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@ public interface Status {

void setClean(boolean isClean);

StatusFormat getFormat();

void setFormat(StatusFormat format);

String getBranchName();

void setBranchName(String branchName);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import static com.google.common.collect.Sets.newConcurrentHashSet;
import static java.nio.file.Files.isDirectory;
import static java.util.Collections.singletonList;
import static org.eclipse.che.api.project.shared.dto.event.GitChangeEventDto.Type.ADDED;
import static org.eclipse.che.api.project.shared.dto.event.GitChangeEventDto.Type.MODIFIED;
import static org.eclipse.che.api.project.shared.dto.event.GitChangeEventDto.Type.UNTRACKED;
Expand All @@ -29,7 +30,6 @@
import org.eclipse.che.api.core.jsonrpc.commons.RequestTransmitter;
import org.eclipse.che.api.git.exception.GitException;
import org.eclipse.che.api.git.shared.Status;
import org.eclipse.che.api.git.shared.StatusFormat;
import org.eclipse.che.api.project.shared.dto.event.GitChangeEventDto;
import org.eclipse.che.api.vfs.watcher.FileWatcherManager;
import org.slf4j.Logger;
Expand Down Expand Up @@ -111,7 +111,7 @@ private Consumer<String> transmitConsumer(String path) {
String project = normalizedPath.split("/")[0];
String itemPath = normalizedPath.substring(normalizedPath.indexOf("/") + 1);
try {
Status status = gitConnectionFactory.getConnection(project).status(StatusFormat.SHORT);
Status status = gitConnectionFactory.getConnection(project).status(singletonList(itemPath));
GitChangeEventDto.Type type;
if (status.getAdded().contains(itemPath)) {
type = ADDED;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import org.eclipse.che.api.git.shared.Revision;
import org.eclipse.che.api.git.shared.ShowFileContentResponse;
import org.eclipse.che.api.git.shared.Status;
import org.eclipse.che.api.git.shared.StatusFormat;
import org.eclipse.che.api.git.shared.Tag;

/**
Expand Down Expand Up @@ -330,13 +329,16 @@ void cloneWithSparseCheckout(String directory, String remoteUrl)
void rm(RmParams params) throws GitException;

/**
* Get status of working tree.
* Get status.
*
* @param format the format of the ouput
* @param filter list of paths to filter the status. Status result will include only files witch
* paths are contained in the filter list, or are children of the folder paths that are
* mentioned in the filter list. Unfiltered status of working tree will be returned, if the
* filter list is empty
* @return status.
* @throws GitException if any error occurs
*/
Status status(StatusFormat format) throws GitException;
Status status(List<String> filter) throws GitException;

/**
* Create new tag.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import static com.google.common.collect.Sets.newConcurrentHashSet;
import static java.nio.file.Files.isDirectory;
import static java.util.Collections.emptyList;
import static org.eclipse.che.api.vfs.watcher.FileWatcherManager.EMPTY_CONSUMER;
import static org.eclipse.che.dto.server.DtoFactory.newDto;
import static org.slf4j.LoggerFactory.getLogger;
Expand All @@ -26,7 +27,6 @@
import org.eclipse.che.api.core.jsonrpc.commons.RequestTransmitter;
import org.eclipse.che.api.git.exception.GitException;
import org.eclipse.che.api.git.shared.Status;
import org.eclipse.che.api.git.shared.StatusFormat;
import org.eclipse.che.api.vfs.watcher.FileWatcherManager;
import org.slf4j.Logger;

Expand Down Expand Up @@ -108,7 +108,7 @@ private Consumer<String> transmitConsumer(String path) {
return id -> {
String project = (path.startsWith("/") ? path.substring(1) : path).split("/")[0];
try {
Status status = gitConnectionFactory.getConnection(project).status(StatusFormat.SHORT);
Status status = gitConnectionFactory.getConnection(project).status(emptyList());
Status statusDto = newDto(Status.class);
statusDto.setAdded(status.getAdded());
statusDto.setUntracked(status.getUntracked());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
import org.eclipse.che.api.git.shared.Revision;
import org.eclipse.che.api.git.shared.ShowFileContentResponse;
import org.eclipse.che.api.git.shared.Status;
import org.eclipse.che.api.git.shared.StatusFormat;
import org.eclipse.che.api.git.shared.Tag;
import org.eclipse.che.api.git.shared.TagCreateRequest;
import org.eclipse.che.api.project.server.FolderEntry;
Expand Down Expand Up @@ -439,9 +438,9 @@ public void reset(ResetRequest request) throws ApiException {
@GET
@Path("status")
@Produces({MediaType.APPLICATION_JSON, MediaType.TEXT_PLAIN})
public Status status(@QueryParam("format") StatusFormat format) throws ApiException {
public Status status(@QueryParam("filter") List<String> filter) throws ApiException {
try (GitConnection gitConnection = getGitConnection()) {
return gitConnection.status(format);
return gitConnection.status(filter);
}
}

Expand Down
Loading

0 comments on commit fc35ba2

Please sign in to comment.