opendev/gerrit
Code Issues Proposed changes

Refactor: Remove duplicate code to handle outdated filter responses

Browse Source 
If a user interface allows filtering and requests the filtered results
via RPC's it is important to ignore outdated results, since the user
may change the filter quickly and responses may come back out-of-order.
If outdated results are not ignored they would overwrite a correct
result that was received before. The code to handle this was repeated
in several places. With this refactoring there is now one
GerritCallback implementation that handles this.

Change-Id: Icfd13c4815a7818b8980aa316d26fb45803aa00a
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin
2013-01-09 07:40:51 +01:00
parent 1b5b6f1d93
commit f1897bbb31
5 changed files with 122 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/GroupListScreen.java
View File

@@ -18,6 +18,8 @@ import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.groups.GroupMap;
import com.google.gerrit.client.rpc.ScreenLoadCallback;
import com.google.gerrit.client.ui.AccountScreen;
import com.google.gerrit.client.ui.FilteredUserInterface;
import com.google.gerrit.client.ui.IgnoreOutdatedFilterResultsCallbackWrapper;
import com.google.gerrit.common.PageLinks;
import com.google.gwt.event.dom.client.KeyUpEvent;
import com.google.gwt.event.dom.client.KeyUpHandler;
@@ -25,7 +27,7 @@ import com.google.gwt.user.client.ui.HorizontalPanel;
import com.google.gwt.user.client.ui.Label;
import com.google.gwtexpui.globalkey.client.NpTextBox;
public class GroupListScreen extends AccountScreen {
public class GroupListScreen extends AccountScreen implements FilteredUserInterface {
private GroupTable groups;
private NpTextBox filterTxt;
private String subname;
@@ -37,25 +39,20 @@ public class GroupListScreen extends AccountScreen {
}
private void refresh() {
final String mySubname = subname;
GroupMap.match(subname, new ScreenLoadCallback<GroupMap>(this) {
@Override
protected void preDisplay(final GroupMap result) {
if ((mySubname == null && subname == null)
|| (mySubname != null && mySubname.equals(subname))) {
display(result);
}
// Else ignore the result, the user has already changed subname and
// the result is not relevant anymore. If multiple RPC's are fired
// the results may come back out-of-order and a non-relevant result
// could overwrite the correct result if not ignored.
}
});
GroupMap.match(subname,
new IgnoreOutdatedFilterResultsCallbackWrapper<GroupMap>(this,
new ScreenLoadCallback<GroupMap>(this) {
@Override
protected void preDisplay(final GroupMap result) {
groups.display(result, subname);
groups.finishDisplay();
}
}));
}
private void display(final GroupMap result) {
groups.display(result, subname);
groups.finishDisplay();
@Override
public String getCurrentFilter() {
return subname;
}
@Override

31
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectListScreen.java
View File

@@ -20,7 +20,9 @@ import com.google.gerrit.client.GitwebLink;
import com.google.gerrit.client.projects.ProjectInfo;
import com.google.gerrit.client.projects.ProjectMap;
import com.google.gerrit.client.rpc.ScreenLoadCallback;
import com.google.gerrit.client.ui.FilteredUserInterface;
import com.google.gerrit.client.ui.HighlightingInlineHyperlink;
import com.google.gerrit.client.ui.IgnoreOutdatedFilterResultsCallbackWrapper;
import com.google.gerrit.client.ui.ProjectSearchLink;
import com.google.gerrit.client.ui.ProjectsTable;
import com.google.gerrit.client.ui.Screen;
@@ -34,7 +36,7 @@ import com.google.gwt.user.client.ui.HorizontalPanel;
import com.google.gwt.user.client.ui.Label;
import com.google.gwtexpui.globalkey.client.NpTextBox;
public class ProjectListScreen extends Screen {
public class ProjectListScreen extends Screen implements FilteredUserInterface {
private ProjectsTable projects;
private NpTextBox filterTxt;
private String subname;
@@ -46,20 +48,19 @@ public class ProjectListScreen extends Screen {
}
private void refresh() {
final String mySubname = subname;
ProjectMap.match(subname, new ScreenLoadCallback<ProjectMap>(this) {
@Override
protected void preDisplay(final ProjectMap result) {
if ((mySubname == null && subname == null)
|| (mySubname != null && mySubname.equals(subname))) {
projects.display(result);
}
// Else ignore the result, the user has already changed subname and
// the result is not relevant anymore. If multiple RPC's are fired
// the results may come back out-of-order and a non-relevant result
// could overwrite the correct result if not ignored.
}
});
ProjectMap.match(subname,
new IgnoreOutdatedFilterResultsCallbackWrapper<ProjectMap>(this,
new ScreenLoadCallback<ProjectMap>(this) {
@Override
protected void preDisplay(final ProjectMap result) {
projects.display(result);
}
}));
}
@Override
public String getCurrentFilter() {
return subname;
}
@Override

25
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/FilteredUserInterface.java Normal file
View File

@@ -0,0 +1,25 @@
// Copyright (C) 2013 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.client.ui;
public interface FilteredUserInterface {
/**
* Return the value by which the user interface is currently filtered.
*
* @return value by which the user interface is currently filtered,
* <code>null</code> or empty String if currently no filter is applied
*/
public String getCurrentFilter();
}

50
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/IgnoreOutdatedFilterResultsCallbackWrapper.java Normal file
View File

@@ -0,0 +1,50 @@
// Copyright (C) 2013 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.client.ui;
import com.google.gerrit.client.rpc.GerritCallback;
/**
* GerritCallback to be used on user interfaces that allow filtering to handle
* RPC's that request filtering. The user may change the filter quickly so that
* a response may be outdated when the client receives it. In this case the
* response must be ignored because the responses to RCP's may come out-of-order
* and an outdated response would overwrite the correct result which was
* received before.
*/
public class IgnoreOutdatedFilterResultsCallbackWrapper<T> extends GerritCallback<T> {
private final FilteredUserInterface filteredUI;
private final String myFilter;
private final GerritCallback<T> cb;
public IgnoreOutdatedFilterResultsCallbackWrapper(
final FilteredUserInterface filteredUI, final GerritCallback<T> cb) {
this.filteredUI = filteredUI;
this.myFilter = filteredUI.getCurrentFilter();
this.cb = cb;
}
@Override
public void onSuccess(final T result) {
if ((myFilter == null && filteredUI.getCurrentFilter() == null)
|| (myFilter != null && myFilter.equals(filteredUI.getCurrentFilter()))) {
cb.onSuccess(result);
}
// Else ignore the result, the user has already changed the filter
// and the result is not relevant anymore. If multiple RPC's are
// fired the results may come back out-of-order and a non-relevant
// result could overwrite the correct result if not ignored.
}
}

37
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ProjectListPopup.java
View File

@@ -36,7 +36,7 @@ import com.google.gwtexpui.globalkey.client.NpTextBox;
import com.google.gwtexpui.user.client.PluginSafeDialogBox;
/** It creates a popup containing all the projects. */
public class ProjectListPopup {
public class ProjectListPopup implements FilteredUserInterface {
private HighlightingProjectsTable projectsTab;
private PluginSafeDialogBox popup;
private NpTextBox filterTxt;
@@ -174,27 +174,22 @@ public class ProjectListPopup {
}
protected void populateProjects() {
final String mySubname = subname;
ProjectMap.match(subname, new GerritCallback<ProjectMap>() {
@Override
public void onSuccess(final ProjectMap result) {
if ((mySubname == null && subname == null)
|| (mySubname != null && mySubname.equals(subname))) {
display(result);
}
// Else ignore the result, the user has already changed subname and
// the result is not relevant anymore. If multiple RPC's are fired
// the results may come back out-of-order and a non-relevant result
// could overwrite the correct result if not ignored.
}
});
ProjectMap.match(subname,
new IgnoreOutdatedFilterResultsCallbackWrapper<ProjectMap>(this,
new GerritCallback<ProjectMap>() {
@Override
public void onSuccess(final ProjectMap result) {
projectsTab.display(result, subname);
if (firstPopupLoad) { // Display was delayed until table was loaded
firstPopupLoad = false;
displayPopup();
}
}
}));
}
private void display(final ProjectMap result) {
projectsTab.display(result, subname);
if (firstPopupLoad) { // Display was delayed until table was loaded
firstPopupLoad = false;
displayPopup();
}
@Override
public String getCurrentFilter() {
return subname;
}
}