opendev/gerrit
Code Issues Proposed changes

Merge "Refactor: Remove duplicate code to handle outdated filter responses"

Browse Source
This commit is contained in:
David Pursehouse
2013-01-09 00:14:37 -08:00
committed by Gerrit Code Review
parent c4cd304fe3 f1897bbb31
commit 9f0092b31e
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;
}
}