Skip to content

Commit c10da6f

Browse files
Merge pull request #403 from appdevforall/refactor/ADFA-5137-setup-complete-stops-existing
ADFA-5137 refactor(state): setup_complete stops existing — the app as…
2 parents 1d571ae + 34d87a4 commit c10da6f

82 files changed

Lines changed: 458 additions & 137 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎controller/app/src/main/java/org/iiab/controller/MainActivity.java‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -264,16 +264,20 @@ public void onReceive(Context context, Intent intent) {
264264
public void onCreate(Bundle savedInstanceState) {
265265
super.onCreate(savedInstanceState);
266266

267-
// Intercept launch and redirect to Setup Wizard if first time
268-
SharedPreferences internalPrefs = getSharedPreferences(getString(R.string.pref_file_internal), Context.MODE_PRIVATE);
269-
if (!internalPrefs.getBoolean(getString(R.string.pref_key_setup_complete), false)) {
267+
// Intercept launch and redirect to the legacy Setup Wizard when there is nothing to run.
268+
// ADFA-5137: asks the device rather than setup_complete, and says which mode it wants. The
269+
// old catch wrote the flag true so this branch would stop firing; with the flag gone there is
270+
// nothing to write, and nothing needs writing — the condition is re-derived every launch, so
271+
// a missing Activity stops being permanent state and goes back to being a log line.
272+
if (!terminalOnlyLaunch(getIntent())
273+
&& !org.iiab.controller.system.data.SystemFactsReader.hereOrOnTheWay(this)) {
270274
try {
271-
startActivity(new Intent(this, SetupActivity.class));
275+
startActivity(new Intent(this, SetupActivity.class)
276+
.putExtra(SetupActivity.EXTRA_WIZARD_MODE, true));
272277
finish();
273278
return; // We stop the execution of MainActivity right here
274279
} catch (android.content.ActivityNotFoundException e) {
275280
android.util.Log.w(TAG, "SetupActivity not found. Skipping initial setup.");
276-
internalPrefs.edit().putBoolean(getString(R.string.pref_key_setup_complete), true).apply();
277281
}
278282
}
279283

@@ -558,6 +562,26 @@ protected void onNewIntent(Intent intent) {
558562
maybeOpenTerminalFromIntent(intent);
559563
}
560564

565+
/**
566+
* ADFA-5137 (review): was this Activity launched only to show the terminal?
567+
*
568+
* <p>Asked before the first-run redirect above, because the order matters and it did not use to.
569+
* That redirect fired on {@code setup_complete}, which was true on any device that had ever
570+
* started an install, so a terminal launch never met it. Now it asks the disk — and with no
571+
* system, Settings → Terminal and the terminal's own keep-alive notification would land in the
572+
* legacy setup shell in wizard mode with Back blocked, having dropped the extras that said what
573+
* they came for.
574+
*
575+
* <p>Read here rather than deferring to {@code maybeOpenTerminalFromIntent}, which runs much later
576+
* in {@code onCreate}: the redirect happens first, so the question has to be answerable first.
577+
*/
578+
private static boolean terminalOnlyLaunch(Intent intent) {
579+
// EXTRA_OPEN_TERMINAL alone, not paired with EXTRA_TERMINAL_ONLY: the redesign's Settings entry
580+
// sets both, but TerminalSessionService's keep-alive notification sets only the first, and both
581+
// came here to open a terminal. What decides the redirect is what the caller came for.
582+
return intent != null && intent.getBooleanExtra(EXTRA_OPEN_TERMINAL, false);
583+
}
584+
561585
/** Open the full terminal when launched from its keep-alive notification (ADFA-4696). */
562586
private void maybeOpenTerminalFromIntent(Intent intent) {
563587
if (intent == null || terminalController == null) return;

‎controller/app/src/main/java/org/iiab/controller/SetupActivity.java‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
*/
99
package org.iiab.controller;
1010

11-
import android.content.Context;
12-
import android.content.SharedPreferences;
1311
import android.os.Bundle;
1412
import android.view.View;
1513

@@ -31,6 +29,14 @@
3129
*/
3230
public class SetupActivity extends AppCompatActivity {
3331

32+
/**
33+
* ADFA-5137: true opens this screen as the first-run wizard, false as Settings. Set by whoever
34+
* opens it, because the mode is a property of the reason for opening it and of nothing else.
35+
* Absent means Settings, which is the safe default: a Settings screen is navigable and a wizard
36+
* blocks Back.
37+
*/
38+
public static final String EXTRA_WIZARD_MODE = "org.iiab.controller.SETUP_WIZARD_MODE";
39+
3440
private boolean wizardMode;
3541

3642
@Override
@@ -39,9 +45,14 @@ protected void onCreate(Bundle savedInstanceState) {
3945
setContentView(R.layout.activity_setup);
4046
org.iiab.controller.help.TooltipWiring.wireAll(getWindow().getDecorView());
4147

42-
SharedPreferences prefs = getSharedPreferences(
43-
getString(R.string.pref_file_internal), Context.MODE_PRIVATE);
44-
wizardMode = !prefs.getBoolean(getString(R.string.pref_key_setup_complete), false);
48+
// ADFA-5137: the caller says which mode this is, because only the caller knows.
49+
//
50+
// It used to read setup_complete, and that was the one reader asking a genuinely different
51+
// question: not "is there a system" but "am I the first-run wizard or am I Settings". Two
52+
// callers open this screen for those two reasons — MainActivity's first-run redirect and its
53+
// Settings button — so the answer belongs in the Intent. Migrating this one to the presence
54+
// rule would have produced a Settings screen that believes it is a wizard.
55+
wizardMode = getIntent() != null && getIntent().getBooleanExtra(EXTRA_WIZARD_MODE, false);
4556

4657
View rail = findViewById(R.id.setup_rail);
4758
if (wizardMode) {

‎controller/app/src/main/java/org/iiab/controller/SetupSectionFragment.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,9 +240,10 @@ private void finishEnrollment(SharedPreferences delivery) {
240240
private void completeSetup() {
241241
// ADFA-4466 Phase 2: setup funnel completion (no-op unless opted in).
242242
org.iiab.controller.analytics.AnalyticsClient.with(requireContext()).logOnboardingCompleted();
243-
SharedPreferences prefs = requireContext().getSharedPreferences(
244-
getString(R.string.pref_file_internal), Context.MODE_PRIVATE);
245-
prefs.edit().putBoolean(getString(R.string.pref_key_setup_complete), true).apply();
243+
// ADFA-5137: nothing to mark. This screen belongs to the legacy setup shell, and it wrote
244+
// setup_complete on its way out — a fourth writer of a flag nobody cleared. LibraryActivity
245+
// now asks the device instead, so if this path really did install a system it will be found,
246+
// and if it did not, the wizard is where the user should land.
246247
startActivity(new Intent(requireContext(), org.iiab.controller.redesign.LibraryActivity.class));
247248
requireActivity().finish();
248249
}

‎controller/app/src/main/java/org/iiab/controller/install/presentation/InstallService.java‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1509,7 +1509,7 @@ private void doCancel() {
15091509
/**
15101510
* ADFA-5119: erase every trace of a system the user decided not to build.
15111511
*
1512-
* <p>Five things claimed that system existed or was about to. Leaving any one of them behind is
1512+
* <p>Four things claimed that system existed or was about to. Leaving any one of them behind is
15131513
* a specific bug, not untidiness:
15141514
*
15151515
* <ol>
@@ -1523,11 +1523,13 @@ private void doCancel() {
15231523
* the content chosen for the tier being given up is not drained into the next one.</li>
15241524
* <li><b>{@code installed_tier}</b>, which a later "Get more" reads to size content against a
15251525
* system that was never installed.</li>
1526-
* <li><b>{@code setup_complete}</b>, which is what decides whether the next launch opens the
1527-
* library or the wizard. This is the one that keeps the promise: no path ends in the app
1528-
* with no system.</li>
15291526
* </ol>
15301527
*
1528+
* <p>There used to be a fifth: {@code setup_complete} → false, which was what kept the promise that
1529+
* no path ends in the app with no system. ADFA-5137 deleted the flag, so the promise is now kept by
1530+
* not having anything to unset — the launch asks whether a rootfs, an install or a deep operation
1531+
* is there, and after this cleanup none of the three is.
1532+
*
15311533
* <p>The install marker is a sixth, and it is already handled — {@link #teardown()} clears it on
15321534
* every clean terminal. Left set, the next launch would open in damaged-system recovery over a
15331535
* system nobody asked for.
@@ -1564,22 +1566,22 @@ private void forgetTheAbandonedSystem() {
15641566
org.iiab.controller.system.data.ContentStateInvalidator.replacementSucceeded(this,
15651567
org.iiab.controller.system.domain.SystemReplacement.Cause.ABANDONED_INSTALL);
15661568

1567-
// 4 + 5. The decision itself, and last on purpose. If the process is killed part-way
1569+
// 4. The recorded tier, and last on purpose. If the process is killed part-way
15681570
// through this method, everything above it is disposable wreckage and the install marker
15691571
// is still set, so the next launch enters damaged-system recovery — which offers a
15701572
// reinstall. Clearing setup_complete first and dying here would instead send the user to
15711573
// the wizard while the marker still says an install is running.
15721574
//
1573-
// commit(), not apply(): the state posted below sends the UI
1574-
// to the tier selection immediately, and a later cold launch reads setup_complete to
1575-
// decide between the library and the wizard. An asynchronous write is a race with both.
1575+
// commit(), not apply(): the state posted below sends the UI to the tier selection
1576+
// immediately, and an asynchronous write would race it.
15761577
//
1577-
// We are the first writer to set setup_complete false — the other four only ever set it
1578-
// true, which is why an abandoned install used to strand the user on an empty library.
1578+
// ADFA-5119 added a setup_complete → false here and called itself the first writer of
1579+
// false among five. ADFA-5137 removed the flag altogether, so there is nothing to unset:
1580+
// the marker cleared by teardown() and the absent rootfs now say the same thing between
1581+
// them, and they cannot disagree with each other the way the flag could disagree with both.
15791582
getSharedPreferences(getString(R.string.pref_file_internal), Context.MODE_PRIVATE)
15801583
.edit()
15811584
.remove("installed_tier")
1582-
.putBoolean(getString(R.string.pref_key_setup_complete), false)
15831585
.commit();
15841586
} catch (Exception e) {
15851587
// Never leave the UI waiting because a cleanup step failed: the state below is what
@@ -1602,7 +1604,7 @@ private void teardown() {
16021604
* no system, so forgetting that an install happened is exactly what let the
16031605
* app open an empty library. Success, cancellation and every module or reset
16041606
* path still pass true — a cancellation has already removed the residue and
1605-
* cleared setup_complete, so it needs no marker to be recovered from.
1607+
* removed the residue and left no rootfs, so it needs no marker to say so.
16061608
*/
16071609
private void teardown(boolean clearMarker) {
16081610
// ADFA-5119: nothing to wait for once this is over — neither the window nor a queued attempt.

‎controller/app/src/main/java/org/iiab/controller/redesign/LibraryActivity.java‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import android.animation.Animator;
44
import android.animation.AnimatorListenerAdapter;
55
import android.content.Intent;
6-
import android.content.SharedPreferences;
76
import android.content.res.Configuration;
87
import android.os.Bundle;
98
import android.os.Handler;
@@ -40,6 +39,23 @@ public class LibraryActivity extends AppCompatActivity implements ServerControll
4039
public static final String EXTRA_INSTALLING = "installing";
4140
/** ADFA-4777: preselect a bottom-nav tab on launch (e.g. from the wizard's "Copy from a phone"). */
4241
public static final String EXTRA_TAB = "tab";
42+
/**
43+
* ADFA-5137: the caller knows there is no system and is bringing the user here to get one.
44+
*
45+
* <p>Only the wizard's "Copy from a phone" sets it. That choice has to land on the Clone tab with
46+
* nothing installed and nothing yet in flight, which is precisely the state that otherwise sends
47+
* the user back to the wizard — so before this ticket the wizard wrote {@code setup_complete} to
48+
* get past the check, and that lie is the entrance to findings 3 and 5.
49+
*
50+
* <p>An Intent extra rather than a stored fact — but its lifetime is the <b>task record</b>, not
51+
* this navigation, and the difference is worth stating because a first draft of this comment got it
52+
* wrong in both directions. Android replays the launching Intent when the process is killed and the
53+
* task is restored, so the extra survives that; swiping the task away is what ends it. And
54+
* {@code onNewIntent} calls {@code setIntent}, so a later arrival carrying no {@code settingUp}
55+
* replaces it. Both outcomes are truthful — the user lands on Home, which since ADFA-5137 has a
56+
* labelled way to install a system — but nobody should read this as "it dies when you navigate".
57+
*/
58+
public static final String EXTRA_SETTING_UP = "settingUp";
4359
private boolean installing = false;
4460

4561
/** ADFA-4799: bottom bar (compact) and rail (medium/expanded) share the NavigationBarView
@@ -97,10 +113,18 @@ public class LibraryActivity extends AppCompatActivity implements ServerControll
97113
protected void onCreate(Bundle savedInstanceState) {
98114
super.onCreate(savedInstanceState);
99115

100-
// Not set up yet? Run the first-run wizard, then it routes back here.
101-
SharedPreferences prefs0 = getSharedPreferences(
102-
getString(R.string.pref_file_internal), MODE_PRIVATE);
103-
if (!prefs0.getBoolean(getString(R.string.pref_key_setup_complete), false)) {
116+
// ADFA-5137: nothing here and nothing coming? Run the first-run wizard, then it routes back.
117+
//
118+
// This used to read setup_complete, a flag written when an install STARTED and cleared by
119+
// nobody — so it could say "set up" while the device had no system, and then this branch
120+
// routed past the wizard forever. That pair is findings 3 and 5 of state-spine.svg. The
121+
// question was never "did setup happen": it is "is there a system, or one on the way", and
122+
// that is answerable from the disk and the two markers, none of which can drift from what
123+
// they describe.
124+
boolean broughtHereToSetUp = getIntent() != null
125+
&& getIntent().getBooleanExtra(EXTRA_SETTING_UP, false);
126+
if (!broughtHereToSetUp
127+
&& !org.iiab.controller.system.data.SystemFactsReader.hereOrOnTheWay(this)) {
104128
startActivity(new Intent(this, WizardActivity.class));
105129
finish();
106130
return;

‎controller/app/src/main/java/org/iiab/controller/redesign/LibraryHomeFragment.java‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,13 @@ public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup c
105105
requireContext(), SetupProgressActivity.class));
106106
return;
107107
}
108+
// ADFA-5137: with no system, the way forward is choosing one — the tier step, the same
109+
// place the wizard sends people. Named, on the header, instead of a sentence pointing
110+
// at a control at the bottom of the screen called "Get more".
111+
if (headerState == H_NO_LIBRARY) {
112+
openGetMore();
113+
return;
114+
}
108115
// ADFA-4837: retry only when it is genuinely safe — canStartServer guards
109116
// against stacking a second proot over a live one.
110117
if (headerState != H_FAILED) return;
@@ -242,6 +249,26 @@ private View makeGetMoreCell(int cardH) {
242249
}
243250

244251
private void openGetMore() {
252+
// ADFA-5137 (review): refuse while a deep operation owns the environment, and refuse HERE so
253+
// both entrances are covered — the footer control and the header button this ticket added.
254+
//
255+
// The hole is not theoretical and the header made it one tap wide. isSystemInstalled() is false
256+
// for the whole time an install marker is set, and a clone-receive holds both the marker and
257+
// the lock — so during a live receive the header reads "no library" and this method would take
258+
// the Step-1 branch. That branch starts an install with reinstall=false, InstallService's
259+
// non-destructive guard sees the half-received rootfs directory, skips the extract and reports
260+
// success, and its teardown clears the marker that a killed receive needs for recovery. That is
261+
// exactly the "boot the wreck" failure InstallService's own cleanup comment warns about,
262+
// reached from a button labelled as a way out.
263+
//
264+
// ownerHeld, not isHeld: a live content download holds no owner marker and must not block this
265+
// (ADFA-4957 draws the same line for the server toggle).
266+
if (org.iiab.controller.env.EnvironmentLock.ownerHeld(requireContext())) {
267+
if (getView() != null) {
268+
org.iiab.controller.util.Snackbars.make(getView(), R.string.k2go_install_busy).show();
269+
}
270+
return;
271+
}
245272
// If a system is already installed, skip the destructive system step and go straight
246273
// to content (Step 2). Otherwise run the full setup from Step 1.
247274
Intent i = new Intent(requireContext(), SetupLibraryActivity.class);
@@ -572,8 +599,19 @@ private void setHeader(int h) {
572599
// app working and gets a spinner; the rest are statements and get nothing. The status
573600
// colour lives on the dot only — the button wears the brand colour, because it is a
574601
// control rather than a severity.
602+
// ADFA-5137: H_NO_LIBRARY gets one too. It was the only state here that offered nothing, and
603+
// that is finding 5 of state-spine.svg: the header said "tap Get more to install" while being
604+
// plain text, pointing at a control at the far bottom of the screen whose name says content
605+
// rather than system. Meanwhile both cards on the way there offer Install and Schedule, and
606+
// both refuse.
607+
//
608+
// ADFA-5137 also closes the way INTO this state, so in principle nobody arrives here any more.
609+
// The button stays anyway, because "in principle" is what the last four dead ends had in
610+
// common: a state with no exit is a bug whoever reaches it, including by a route that does not
611+
// exist yet. One line in a switch that already hands out two other buttons.
575612
int action = h == H_FAILED ? R.string.k2go_home_retry
576-
: h == H_INSTALLING ? R.string.k2go_home_see_progress : 0;
613+
: h == H_INSTALLING ? R.string.k2go_home_see_progress
614+
: h == H_NO_LIBRARY ? R.string.k2go_home_install_system : 0;
577615
if (homeStatusAction != null) {
578616
homeStatusAction.setVisibility(action != 0 ? View.VISIBLE : View.GONE);
579617
if (action != 0) homeStatusAction.setText(action);

‎controller/app/src/main/java/org/iiab/controller/redesign/ModuleHubFragment.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,12 @@ public class ModuleHubFragment extends Fragment {
6565
* with. The screen still listed all six as installable, because it decided that from a probe
6666
* — and with no system nothing answers, so everything looked missing.
6767
*
68-
* <p>Reachable, not theoretical: {@code pref_key_setup_complete} is only ever written true —
69-
* five writers, none of them clears it — so after a reset or a failed restore the app still
70-
* routes to the Library, and Settings still opens this screen over an empty rootfs.
68+
* <p>Reachable, not theoretical — and it stays reachable after ADFA-5137. That ticket removed the
69+
* flag this note used to blame ({@code setup_complete}, written true by four sites and cleared by
70+
* none), so a device that simply has no system now opens the wizard instead of the Library. What
71+
* still lands here is the case where a marker is held: a failed restore keeps it, so the launch
72+
* treats the device as having something on the way, reaches the Library, and this screen can be
73+
* opened over a rootfs that is empty or half-written. The precondition still has to be asked here.
7174
*
7275
* <p>Seeded true so the first frame looks like the ordinary case; the background pass
7376
* corrects it before anything is offered.

0 commit comments

Comments
 (0)