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

SWC-7126 - Fix bootstrapping redirect bug #5560

Merged
merged 1 commit into from
Oct 23, 2024
Merged

SWC-7126 - Fix bootstrapping redirect bug #5560

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
92 changes: 59 additions & 33 deletions src/main/java/org/sagebionetworks/web/client/Portal.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.gwt.user.client.ui.RootPanel;
import com.google.gwt.user.client.ui.SimplePanel;
import java.util.HashMap;
import java.util.function.Consumer;
import org.sagebionetworks.schema.adapter.JSONObjectAdapter;
import org.sagebionetworks.web.client.mvp.AppActivityMapper;
import org.sagebionetworks.web.client.mvp.AppPlaceHistoryMapper;
Expand Down Expand Up @@ -92,16 +93,12 @@ public void onSuccess() {
ginjector
.getSynapseProperties()
.getInitSynapsePropertiesFuture();
FluentFuture<Void> checkForUserChangeFuture = ginjector
.getAuthenticationController()
.getCheckForUserChangeFuture();

FluentFuture
.from(
whenAllComplete(
featureFlagConfigFuture,
synapsePropertiesFuture,
checkForUserChangeFuture
Copy link
Contributor Author

@nickgros nickgros Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We have a dependency cycle where AuthenticationControllerImpl.checkForUserChangeFuture may try to call GlobalApplicationStateImpl.goTo, but GlobalApplicationStateImpl.goTo relies on the PlaceController, which is not ready; it is initialized and set in this future's callback (on line 167 (old) / 164 (new).

We'll run the first two futures, but defer running checkForUserChangeFuture until the placeController is set

synapsePropertiesFuture
)
.call(
() -> {
Expand Down Expand Up @@ -170,38 +167,67 @@ public void onSuccess() {
globalApplicationState.setAppPlaceHistoryMapper(
historyMapper
);
globalApplicationState.init(
new Callback() {
@Override
public void invoke() {
// listen for window close (or navigating away)
registerWindowClosingHandler(
globalApplicationState
);
registerOnPopStateHandler(globalApplicationState);

// start version timer
ginjector.getVersionTimer().start();
// start timer to check for Synapse outage or scheduled maintenance
ginjector.getSynapseStatusDetector().start();
// Goes to place represented on URL or default place
historyHandler.handleCurrentHistory();
globalApplicationState.initializeDropZone();
globalApplicationState.initializeToastContainer();
// initialize the view default columns so that they're ready when we need them (do this by constructing that singleton object)
ginjector.getViewDefaultColumns();
FluentFuture
.from(
whenAllComplete(
// Checking the session may result in a redirect, so this must be invoked after `setPlaceController`
ginjector
.getAuthenticationController()
.getCheckForUserChangeFuture()
)
Comment on lines +173 to +178
Copy link
Contributor Author

@nickgros nickgros Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

using this whenAllComplete.call pattern just the like the outer future so we can call the wrapping onFailure method if this fails.

.call(
() -> {
globalApplicationState.init(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finally, call globalApplicationState.init

new Callback() {
@Override
public void invoke() {
// listen for window close (or navigating away)
registerWindowClosingHandler(
globalApplicationState
);
registerOnPopStateHandler(
globalApplicationState
);

// start timer to check for user session state change (session expired, or user explicitly logged
// out). Backend endpoints must be set before starting this (because it attempts to get "my user profile")
ginjector.getSessionDetector().start();
// start version timer
ginjector.getVersionTimer().start();
// start timer to check for Synapse outage or scheduled maintenance
ginjector
.getSynapseStatusDetector()
.start();
// Goes to place represented on URL or default place
historyHandler.handleCurrentHistory();
globalApplicationState.initializeDropZone();
globalApplicationState.initializeToastContainer();
// initialize the view default columns so that they're ready when we need them (do this by constructing that singleton object)
ginjector.getViewDefaultColumns();

// start a timer to check to see if we're approaching the max allowable space in the web storage.
// clears out the web storage (cache) if this is the case.
ginjector.getWebStorageMaxSizeDetector().start();
}
}
);
// start timer to check for user session state change (session expired, or user explicitly logged
// out). Backend endpoints must be set before starting this (because it attempts to get "my user profile")
ginjector.getSessionDetector().start();

// start a timer to check to see if we're approaching the max allowable space in the web storage.
// clears out the web storage (cache) if this is the case.
ginjector
.getWebStorageMaxSizeDetector()
.start();
}
}
);
return null;
},
directExecutor()
)
)
.catching(
Throwable.class,
e -> {
onFailure(e);
return null;
},
directExecutor()
);
return null;
},
directExecutor()
Expand Down