Skip to content

Commit 724ef4a

Browse files
committed
Makes long-running Apps tool APIs run properly in background (BL-16350)
Several Apps tool APIs, especially Build, were made to kick off a background task and then return; task completion signaled by websocket. Enhanced so if you return to the tool and build is still running, it gets back into a state indicating the progress of the build. Also, Build is now more appropriately enabled; you don't have to do Customize first if all the required settings are already known.
1 parent 8e709b1 commit 724ef4a

8 files changed

Lines changed: 589 additions & 316 deletions

File tree

src/BloomBrowserUI/publish/Apps/AppPublisherScreen.tsx

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -460,11 +460,7 @@ const AppPublisherScreenContents: React.FunctionComponent<{
460460
<Step expanded={true} completed={false}>
461461
<StepLabel>
462462
<AppActionButton
463-
enabled={
464-
prepareIsReady &&
465-
buildIsNeeded &&
466-
canRunBuild
467-
}
463+
enabled={prepareIsReady && canRunBuild}
468464
l10nKey="PublishTab.Apps.Build"
469465
onClick={() =>
470466
screenState.runAction("build")

src/BloomBrowserUI/publish/Apps/appBuilderShared.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ export interface IAppBuilderStatus {
3434
rabRoot?: string;
3535
trackedBookTitles?: string[];
3636
trackedBooks: IAppBuilderTrackedBook[];
37+
/** The action currently running on the server, or undefined if idle. */
38+
activeAction?: AppBuilderAction;
39+
/** Last progress stage reported by the server (e.g. "building-android-app"). */
40+
activeActionProgressStage?: string;
41+
/** Last progress percent (0–100) reported by the server. */
42+
activeActionProgressPercent?: number;
3743
}
3844

3945
export type AppBuilderPrepareStepId =
@@ -74,6 +80,12 @@ export interface IAppBuilderStatusApi {
7480
apkSizeBytes?: number;
7581
rabRoot?: string;
7682
trackedBookTitles?: string[];
83+
activeAction?: string;
84+
activeActionProgressStage?: string;
85+
activeActionProgressPercent?: number;
86+
ActiveAction?: string;
87+
ActiveActionProgressStage?: string;
88+
ActiveActionProgressPercent?: number;
7789
RabInstalled?: boolean;
7890
ProjectExists?: boolean;
7991
ApkExists?: boolean;
@@ -129,6 +141,10 @@ export interface IProgressStageLabels {
129141

130142
export const kAppBuilderWebSocketContext = "publish-rab";
131143

144+
/// Websocket event id sent by the C# side when a prepare/build/install completes.
145+
/// The message payload is "{action}:success" or "{action}:failure".
146+
export const kAppBuilderActionCompleteEventId = "actionComplete";
147+
132148
export type AppBuilderAction = "prepare" | "build" | "install";
133149

134150
export const defaultStatus: IAppBuilderStatus = {
@@ -209,6 +225,16 @@ export function normalizeStatus(
209225
getDefaultPrepareSteps()
210226
).map(normalizePrepareStepStatus);
211227

228+
const rawActiveAction =
229+
status?.activeAction ?? status?.ActiveAction ?? undefined;
230+
const rawProgressStage =
231+
status?.activeActionProgressStage ??
232+
status?.ActiveActionProgressStage ??
233+
undefined;
234+
const rawProgressPercent =
235+
status?.activeActionProgressPercent ??
236+
status?.ActiveActionProgressPercent ??
237+
undefined;
212238
return {
213239
rabInstalled: status?.rabInstalled ?? status?.RabInstalled ?? false,
214240
projectExists: status?.projectExists ?? status?.ProjectExists ?? false,
@@ -230,6 +256,9 @@ export function normalizeStatus(
230256
trackedBooks: (status?.trackedBooks ?? status?.TrackedBooks ?? []).map(
231257
normalizeTrackedBook,
232258
),
259+
activeAction: rawActiveAction as AppBuilderAction | undefined,
260+
activeActionProgressStage: rawProgressStage,
261+
activeActionProgressPercent: rawProgressPercent,
233262
};
234263
}
235264

src/BloomBrowserUI/publish/Apps/useAppBuilderPublisherScreen.ts

Lines changed: 151 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import * as React from "react";
2-
import { get, getWithPromise, postData, postJson } from "../../utils/bloomApi";
2+
import {
3+
get,
4+
getWithPromise,
5+
post,
6+
postData,
7+
postJson,
8+
} from "../../utils/bloomApi";
39
import {
410
IBloomWebSocketProgressEvent,
511
useSubscribeToWebSocketForEvent,
@@ -16,6 +22,7 @@ import {
1622
IAppBuilderStatusApi,
1723
IAppBuilderSizeEstimatesApi,
1824
kAppBuilderWebSocketContext,
25+
kAppBuilderActionCompleteEventId,
1926
normalizeSizeEstimates,
2027
normalizeStatus,
2128
AppBuilderAction,
@@ -158,6 +165,34 @@ export function useAppBuilderPublisherScreen(
158165
statusRetryTimeoutRef.current = undefined;
159166
}
160167
setStatus(nextStatus);
168+
// If the backend reports an action is running but we have no local busyAction
169+
// (e.g. after a remount mid-build), restore the action and its last-known progress
170+
// so the indicator appears immediately without waiting for a websocket event.
171+
if (nextStatus.activeAction) {
172+
const isRestoringFromBlank = !busyActionRef.current;
173+
setBusyAction(
174+
(current) =>
175+
current ??
176+
(nextStatus.activeAction as AppBuilderAction),
177+
);
178+
if (isRestoringFromBlank) {
179+
if (nextStatus.activeActionProgressStage) {
180+
setProgressStageCode(
181+
nextStatus.activeActionProgressStage,
182+
);
183+
}
184+
setProgressPercent(
185+
nextStatus.activeActionProgressPercent ?? 0,
186+
);
187+
}
188+
} else if (busyActionRef.current) {
189+
// Server is idle but client still thinks an action is running —
190+
// recover by clearing the stale busy state (e.g. after a remount
191+
// that missed the actionComplete websocket event).
192+
setBusyAction(undefined);
193+
setProgressPercent(0);
194+
setProgressStageCode(undefined);
195+
}
161196

162197
if (initializeSettingsAfterPrepare && nextStatus.appDefPath) {
163198
const initializedSettings = await initializeAppBuilderSettings(
@@ -198,18 +233,77 @@ export function useAppBuilderPublisherScreen(
198233
}
199234
}
200235

201-
// This effect is warranted because tab activation is an external UI lifecycle boundary,
202-
// and we need to reset the ephemeral BloomPUB cache plus re-read the RAB project whenever the Apps screen becomes active.
236+
// Track busyAction in a ref so async callbacks can read the latest value
237+
// without closing over a stale render's state.
238+
const busyActionRef = React.useRef(busyAction);
239+
busyActionRef.current = busyAction;
240+
241+
// This effect is warranted because tab activation is an external UI lifecycle boundary.
242+
// We fetch status first so any active build, its current stage, and its progress are
243+
// shown immediately — without waiting for the next websocket event — and so we know
244+
// whether it's safe to reset the ephemeral BloomPUB cache.
203245
React.useEffect(() => {
204246
if (!isActive) {
205247
return;
206248
}
207249

208-
void postData("publish/rab/reset-bloompub-cache", {}).then(() => {
250+
void (async () => {
251+
// Snapshot the current server state right away.
252+
let activeActionFromServer: AppBuilderAction | undefined;
253+
try {
254+
const nextStatus = await fetchStatusAsync();
255+
if (!isMountedRef.current) {
256+
return;
257+
}
258+
setStatus(nextStatus);
259+
activeActionFromServer = nextStatus.activeAction;
260+
if (activeActionFromServer) {
261+
// Only restore server-side progress when we weren't already tracking
262+
// the action locally — if busyAction is already set, live websocket
263+
// events have fresher values than the status snapshot might.
264+
const wasAlreadyTracking = !!busyActionRef.current;
265+
setBusyAction(
266+
(current) => current ?? activeActionFromServer!,
267+
);
268+
if (!wasAlreadyTracking) {
269+
if (nextStatus.activeActionProgressStage) {
270+
setProgressStageCode(
271+
nextStatus.activeActionProgressStage,
272+
);
273+
}
274+
setProgressPercent(
275+
nextStatus.activeActionProgressPercent ?? 0,
276+
);
277+
}
278+
}
279+
void refreshSettings(nextStatus.appDefPath);
280+
} catch {
281+
// Status fetch failed; fall through to the cache-reset path which
282+
// will call refreshStatus() for its own retry handling.
283+
}
284+
285+
if (!isMountedRef.current) {
286+
return;
287+
}
288+
refreshSizeEstimates();
289+
290+
if (activeActionFromServer) {
291+
// A background action is running — skip the cache reset to avoid
292+
// deleting BloomPUBs the build is actively using.
293+
return;
294+
}
295+
296+
// No active action: clear the stale per-session BloomPUB cache, then
297+
// do a full status refresh to pick up any changes since the last visit.
298+
await postData("publish/rab/reset-bloompub-cache", {});
299+
if (!isMountedRef.current) {
300+
return;
301+
}
209302
void refreshStatus();
210303
refreshSizeEstimates();
211-
});
212-
// refreshStatus and refreshSizeEstimates are intentionally omitted so this only reruns when the tab activation boundary changes.
304+
})();
305+
// fetchStatusAsync, refreshSettings, refreshSizeEstimates, and refreshStatus
306+
// are intentionally omitted — this only reruns at the tab-activation boundary.
213307
// eslint-disable-next-line react-hooks/exhaustive-deps
214308
}, [isActive]);
215309

@@ -265,24 +359,30 @@ export function useAppBuilderPublisherScreen(
265359
return;
266360
}
267361

362+
// We know the action is done — clear busy state immediately rather than
363+
// delegating to refreshStatus's recovery else-branch (which is for remounts
364+
// that missed the completion event).
365+
setBusyAction(undefined);
268366
setProgressPercent(0);
269367
setProgressStageCode(undefined);
270368
if (action === "prepare" || action === "build") {
271369
setPendingBuildNeeded(false);
272370
refreshSizeEstimates();
273371
}
372+
// Fetch updated server state: APK path, project existence, build signature,
373+
// and (for prepare) initialize settings from the new appDef.
274374
await refreshStatus(initializeSettingsAfterPrepare);
275-
if (!isMountedRef.current) {
276-
return;
277-
}
278-
279-
setBusyAction(undefined);
280375
}
281376

282377
function runAction(action: AppBuilderAction): void {
283378
if (action === "build" && !hasRequiredBuildSettings(settings)) {
284379
return;
285380
}
381+
// UI guards (canRunBuild, canRunPrepare …) should prevent this, but bail out
382+
// here before touching any shared state as a safety net.
383+
if (busyAction) {
384+
return;
385+
}
286386

287387
getActionLogController(action).clear();
288388

@@ -298,18 +398,48 @@ export function useAppBuilderPublisherScreen(
298398
: undefined,
299399
);
300400
setBusyAction(action);
301-
postData(
302-
`publish/rab/${action}`,
303-
{},
304-
() => {
305-
void handleActionCompleted(action, action === "prepare");
306-
},
307-
() => {
308-
void handleActionCompleted(action, false);
309-
},
310-
);
401+
// The endpoint returns immediately; actual completion arrives via the
402+
// "actionComplete" websocket event handled by the subscription below.
403+
// Pass a failure callback so that a server-side rejection (e.g. another action
404+
// already running) doesn't leave busyAction permanently set.
405+
post(`publish/rab/${action}`, undefined, () => {
406+
// Don't think this can ever happen, since the only immediate failure mode is
407+
// if we try to start an action while another is running, and the UI should
408+
// prevent that. If it does, clear everything so at least the user can try again.
409+
setBusyAction(undefined);
410+
setProgressPercent(0);
411+
setProgressStageCode(undefined);
412+
});
311413
}
312414

415+
// Listen for background-task completion from the server.
416+
// useWebSocketListener keeps the callback reference current on every render
417+
// so handleActionCompleted always has a fresh closure.
418+
useSubscribeToWebSocketForStringMessage(
419+
kAppBuilderWebSocketContext,
420+
kAppBuilderActionCompleteEventId,
421+
(result) => {
422+
const separatorIndex = result.indexOf(":");
423+
if (separatorIndex < 0) {
424+
return;
425+
}
426+
const completedAction = result.substring(
427+
0,
428+
separatorIndex,
429+
) as AppBuilderAction;
430+
// guard against possibility of receiving a completion event that is stale somehow.
431+
if (completedAction !== busyActionRef.current) {
432+
return;
433+
}
434+
const succeeded =
435+
result.substring(separatorIndex + 1) === "success";
436+
void handleActionCompleted(
437+
completedAction,
438+
completedAction === "prepare" && succeeded,
439+
);
440+
},
441+
);
442+
313443
function showApkInExplorerInShell(): void {
314444
if (!status.apkPath) {
315445
return;

src/BloomBrowserUI/react_components/requiresSubscription.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ export const RequiresSubscriptionOverlayWrapper: React.FunctionComponent<{
165165
>
166166
{props.children}
167167
</div>
168-
{featureStatus?.enabled || (
168+
{featureStatus === undefined || featureStatus.enabled || (
169169
<div
170170
css={css`
171171
position: absolute;

src/BloomExe/Publish/Rab/RabProjectModels.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,26 @@ public class RabProjectStatus
2828
public RabTrackedBookInfo[] TrackedBooks { get; set; } = Array.Empty<RabTrackedBookInfo>();
2929
public RabPrepareStepStatus[] PrepareSteps { get; set; } =
3030
Array.Empty<RabPrepareStepStatus>();
31+
32+
/// <summary>
33+
/// The action currently running ("prepare", "build", or "install"), or null if idle.
34+
/// Lets the UI restore its busy indicator when switching back to the Apps tab mid-action.
35+
/// </summary>
36+
public string ActiveAction { get; set; }
37+
38+
/// <summary>
39+
/// The most-recently reported progress stage code for the active action (e.g.
40+
/// "building-android-app"), or null when idle. Sent with the status so the UI can
41+
/// show the correct stage label immediately on tab re-activation, without waiting for
42+
/// the next websocket progress event.
43+
/// </summary>
44+
public string ActiveActionProgressStage { get; set; }
45+
46+
/// <summary>
47+
/// The most-recently reported progress percentage (0–100) for the active action, or
48+
/// 0 when idle. Paired with <see cref="ActiveActionProgressStage"/> for the same reason.
49+
/// </summary>
50+
public int ActiveActionProgressPercent { get; set; }
3151
}
3252

3353
/// <summary>

0 commit comments

Comments
 (0)