From f0082c206824ad2518e6fcd03c24d60dc0c5e471 Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Fri, 18 Sep 2020 11:44:00 +1200 Subject: [PATCH] change controller api to return a view, rather than setting it --- console_conf/controllers/chooser.py | 9 ++-- console_conf/controllers/identity.py | 12 +++-- .../controllers/tests/test_chooser.py | 22 ++++----- console_conf/controllers/welcome.py | 5 +- subiquity/controllers/filesystem.py | 48 +++++++++---------- subiquity/controllers/identity.py | 4 +- subiquity/controllers/installprogress.py | 6 +-- subiquity/controllers/keyboard.py | 5 +- subiquity/controllers/mirror.py | 4 +- subiquity/controllers/proxy.py | 4 +- subiquity/controllers/reboot.py | 3 +- subiquity/controllers/refresh.py | 4 +- subiquity/controllers/snaplist.py | 4 +- subiquity/controllers/ssh.py | 4 +- subiquity/controllers/welcome.py | 5 +- subiquity/controllers/zdev.py | 2 +- subiquity/ui/views/filesystem/filesystem.py | 2 +- subiquitycore/controllers/network.py | 4 +- subiquitycore/core.py | 2 +- subiquitycore/tui.py | 2 +- subiquitycore/tuicontroller.py | 13 ++--- 21 files changed, 78 insertions(+), 86 deletions(-) diff --git a/console_conf/controllers/chooser.py b/console_conf/controllers/chooser.py index 6b92175c..c2618996 100644 --- a/console_conf/controllers/chooser.py +++ b/console_conf/controllers/chooser.py @@ -47,7 +47,7 @@ class RecoveryChooserController(RecoveryChooserBaseController): # one of the current views self._current_view = None - def start_ui(self): + def make_ui(self): # current view is preserved, so going back comes back to the right # screen if self._current_view is None: @@ -57,7 +57,7 @@ class RecoveryChooserController(RecoveryChooserBaseController): if self._model_view is not None: self._current_view = self._model_view - self.ui.set_body(self._current_view) + return self._current_view def _make_views(self): current_view = None @@ -89,9 +89,8 @@ class RecoveryChooserController(RecoveryChooserBaseController): class RecoveryChooserConfirmController(RecoveryChooserBaseController): - def start_ui(self): - view = ChooserConfirmView(self, self.model.selection) - self.ui.set_body(view) + def make_ui(self): + return ChooserConfirmView(self, self.model.selection) def confirm(self): log.warning("user action %s", self.model.selection) diff --git a/console_conf/controllers/identity.py b/console_conf/controllers/identity.py index cf1b9eb6..f3f8a81e 100644 --- a/console_conf/controllers/identity.py +++ b/console_conf/controllers/identity.py @@ -147,8 +147,7 @@ class IdentityController(TuiController): super().__init__(app) self.model = app.base_model.identity - def start_ui(self): - self.ui.set_body(IdentityView(self.model, self)) + def make_ui(self): if get_managed(): device_owner = get_device_owner() if device_owner: @@ -158,7 +157,9 @@ class IdentityController(TuiController): cp = run_command(['ssh-keygen', '-lf', key_file]) self.model.user.fingerprints = ( cp.stdout.replace('\r', '').splitlines()) - self.login() + return self.make_login_view() + else: + return IdentityView(self.model, self) def identity_done(self, email): if self.opts.dry_run: @@ -199,7 +200,7 @@ class IdentityController(TuiController): if self.model.user is None: self.app.prev_screen() - def login(self): + def make_login_view(self): title = "Configuration Complete" self.ui.set_header(title) @@ -208,7 +209,8 @@ class IdentityController(TuiController): login_view = LoginView(self.opts, self.model, self, ifaces) login_view._w.focus_position = 2 - self.ui.set_body(login_view) + def login(self): + self.ui.set_body(self.make_login_view()) def login_done(self): if not self.opts.dry_run: diff --git a/console_conf/controllers/tests/test_chooser.py b/console_conf/controllers/tests/test_chooser.py index d38e63e8..f43fc789 100644 --- a/console_conf/controllers/tests/test_chooser.py +++ b/console_conf/controllers/tests/test_chooser.py @@ -120,7 +120,7 @@ class TestChooserConfirmController(unittest.TestCase): c = RecoveryChooserConfirmController(app) c.model = make_model() c.model.select(c.model.systems[0], c.model.systems[0].actions[0]) - c.start_ui() + c.make_ui() ccv.assert_called() @@ -146,13 +146,12 @@ class TestChooserController(unittest.TestCase): def test_current_ui_first(self, cv, ccsv): app = make_app(model=make_model()) c = RecoveryChooserController(app) - c.ui.start_ui = mock.Mock() # current system view is constructed ccsv.assert_called_with(c, c.model.current, has_more=True) # as well as all systems view cv.assert_called_with(c, c.model.systems) - c.start_ui() - c.ui.set_body.assert_called_with('current') + v = c.make_ui() + self.assertEqual(v, 'current') # user selects more options and the view is replaced c.more_options() c.ui.set_body.assert_called_with('all') @@ -164,13 +163,12 @@ class TestChooserController(unittest.TestCase): def test_current_current_all_there_and_back(self, cv, ccsv): app = make_app(model=make_model()) c = RecoveryChooserController(app) - c.ui.start_ui = mock.Mock() # sanity ccsv.assert_called_with(c, c.model.current, has_more=True) cv.assert_called_with(c, c.model.systems) - c.start_ui() - c.ui.set_body.assert_called_with('current') + v = c.make_ui() + self.assertEqual(v, 'current') # user selects more options and the view is replaced c.more_options() c.ui.set_body.assert_called_with('all') @@ -189,13 +187,12 @@ class TestChooserController(unittest.TestCase): model = RecoverySystemsModel.from_systems([model2_current]) app = make_app(model=model) c = RecoveryChooserController(app) - c.ui.start_ui = mock.Mock() # both views are constructed ccsv.assert_called_with(c, c.model.current, has_more=False) cv.assert_called_with(c, c.model.systems) - c.start_ui() - c.ui.set_body.assert_called_with('current') + v = c.make_ui() + self.assertEqual(v, 'current') # going back does nothing c.back() c.ui.set_body.not_called() @@ -208,7 +205,6 @@ class TestChooserController(unittest.TestCase): model = RecoverySystemsModel.from_systems([model1_non_current]) app = make_app(model=model) c = RecoveryChooserController(app) - c.ui.start_ui = mock.Mock() # sanity self.assertIsNone(c.model.current) @@ -218,5 +214,5 @@ class TestChooserController(unittest.TestCase): # current system view is not constructed at all ccsv.assert_not_called() - c.start_ui() - c.ui.set_body.assert_called_with('all') + v = c.make_ui() + self.assertEqual(v, 'all') diff --git a/console_conf/controllers/welcome.py b/console_conf/controllers/welcome.py index 03654e14..085c6ded 100644 --- a/console_conf/controllers/welcome.py +++ b/console_conf/controllers/welcome.py @@ -22,9 +22,8 @@ class WelcomeController(TuiController): welcome_view = WelcomeView - def start_ui(self): - view = self.welcome_view(self) - self.ui.set_body(view) + def make_ui(self): + return self.welcome_view(self) def done(self): self.app.next_screen() diff --git a/subiquity/controllers/filesystem.py b/subiquity/controllers/filesystem.py index 4138a3ef..ac0d6d9b 100644 --- a/subiquity/controllers/filesystem.py +++ b/subiquity/controllers/filesystem.py @@ -222,15 +222,17 @@ class FilesystemController(SubiquityTuiController): await self._start_task await self._probe_task.wait() if isinstance(self.ui.body, SlowProbing): - self.start_ui() + self.ui.set_body(self.make_ui()) - def start_ui(self): + def make_ui(self): if self._probe_task.task is None or not self._probe_task.task.done(): - self.ui.set_body(SlowProbing(self)) schedule_task(self._wait_for_probing()) + return SlowProbing(self) elif True in self._crash_reports: - self.ui.set_body(ProbingFailed(self)) - self.ui.body.show_error() + pr = self._crash_reports[True] + if pr is not None: + self.app.show_error_report(pr) + return ProbingFailed(self) else: # Once we've shown the filesystem UI, we stop listening for udev # events as merging system changes with configuration the user has @@ -238,20 +240,24 @@ class FilesystemController(SubiquityTuiController): # not today. self.convert_autoinstall_config() self.stop_listening_udev() - self.ui.set_body(GuidedDiskSelectionView(self)) pr = self._crash_reports.get(False) if pr is not None: self.app.show_error_report(pr) - if self.answers['guided']: - disk = self.model.all_disks()[self.answers['guided-index']] - method = self.answers.get('guided-method') - self.ui.body.form.guided_choice.value = { - 'disk': disk, - 'use_lvm': method == "lvm", - } - self.ui.body.done(self.ui.body.form) - elif self.answers['manual']: - self.manual() + if self.answers: + self.app.aio_loop.call_soon(self._start_answers) + return GuidedDiskSelectionView(self) + + def _start_answers(self): + if self.answers['guided']: + disk = self.model.all_disks()[self.answers['guided-index']] + method = self.answers.get('guided-method') + self.ui.body.form.guided_choice.value = { + 'disk': disk, + 'use_lvm': method == "lvm", + } + self.ui.body.done(self.ui.body.form) + elif self.answers['manual']: + self.manual() def run_answers(self): # Handled above as we only want to run answers when probing @@ -366,14 +372,8 @@ class FilesystemController(SubiquityTuiController): self._run_iterator(self._run_actions(self.answers['manual'])) self.answers['manual'] = [] - def guided(self, method): - v = GuidedDiskSelectionView(self.model, self, method) - self.ui.set_body(v) - if self.answers['guided']: - index = self.answers['guided-index'] - v.form.guided.value = True - v.form.guided_choice.disk.widget.index = index - v.form._emit('done') + def guided(self): + self.ui.set_body(GuidedDiskSelectionView(self)) def reset(self): log.info("Resetting Filesystem model") diff --git a/subiquity/controllers/identity.py b/subiquity/controllers/identity.py index e6eaa11a..33a85153 100644 --- a/subiquity/controllers/identity.py +++ b/subiquity/controllers/identity.py @@ -50,8 +50,8 @@ class IdentityController(SubiquityTuiController): if 'user-data' not in self.app.autoinstall_config: raise Exception("no identity data provided") - def start_ui(self): - self.ui.set_body(IdentityView(self.model, self)) + def make_ui(self): + return IdentityView(self.model, self) def run_answers(self): if all(elem in self.answers for elem in diff --git a/subiquity/controllers/installprogress.py b/subiquity/controllers/installprogress.py index d2a13716..9e979ec0 100644 --- a/subiquity/controllers/installprogress.py +++ b/subiquity/controllers/installprogress.py @@ -152,7 +152,7 @@ class InstallProgressController(SubiquityTuiController): self.progress_view.finish_all() self.progress_view.set_status(('info_error', _("An error has occurred"))) - self.start_ui() + self.ui.set_body(self.make_ui()) if crash_report is not None: self.progress_view.show_error(crash_report) @@ -440,7 +440,7 @@ class InstallProgressController(SubiquityTuiController): def click_reboot(self): schedule_task(self._click_reboot()) - def start_ui(self): + def make_ui(self): if self.install_state in [ InstallState.NOT_STARTED, InstallState.RUNNING, @@ -451,8 +451,8 @@ class InstallProgressController(SubiquityTuiController): elif self.install_state == InstallState.ERROR: self.progress_view.title = ( _('An error occurred during installation')) - self.ui.set_body(self.progress_view) schedule_task(self.move_on()) + return self.progress_view def run_answers(self): pass diff --git a/subiquity/controllers/keyboard.py b/subiquity/controllers/keyboard.py index aa5078cd..eda3bc21 100644 --- a/subiquity/controllers/keyboard.py +++ b/subiquity/controllers/keyboard.py @@ -61,11 +61,10 @@ class KeyboardController(SubiquityTuiController): log.debug("loading launguage %s", code) self.model.load_language(code) - def start_ui(self): + def make_ui(self): if self.model.current_lang is None: self.model.load_language('C') - view = KeyboardView(self.model, self, self.opts) - self.ui.set_body(view) + return KeyboardView(self.model, self, self.opts) def run_answers(self): if 'layout' in self.answers: diff --git a/subiquity/controllers/mirror.py b/subiquity/controllers/mirror.py index 72f92764..446bbf0d 100644 --- a/subiquity/controllers/mirror.py +++ b/subiquity/controllers/mirror.py @@ -122,9 +122,9 @@ class MirrorController(SubiquityTuiController): self.check_state = CheckState.DONE self.model.set_country(cc) - def start_ui(self): + def make_ui(self): self.check_state = CheckState.DONE - self.ui.set_body(MirrorView(self.model, self)) + return MirrorView(self.model, self) def run_answers(self): if 'mirror' in self.answers: diff --git a/subiquity/controllers/proxy.py b/subiquity/controllers/proxy.py index ab9bfabd..5433f616 100644 --- a/subiquity/controllers/proxy.py +++ b/subiquity/controllers/proxy.py @@ -48,8 +48,8 @@ class ProxyController(SubiquityTuiController): # by everything; don't have a way to do that today. pass - def start_ui(self): - self.ui.set_body(ProxyView(self.model, self)) + def make_ui(self): + return ProxyView(self.model, self) def run_answers(self): if 'proxy' in self.answers: diff --git a/subiquity/controllers/reboot.py b/subiquity/controllers/reboot.py index 04ea89d5..b048c6ae 100644 --- a/subiquity/controllers/reboot.py +++ b/subiquity/controllers/reboot.py @@ -74,8 +74,9 @@ class RebootController(SubiquityTuiController): await self.app.controllers.InstallProgress.reboot_clicked.wait() self.reboot() - def start_ui(self): + def make_ui(self): schedule_task(self._run()) + return self.ui.body def cancel(self): pass diff --git a/subiquity/controllers/refresh.py b/subiquity/controllers/refresh.py index caff0273..d142ebbc 100644 --- a/subiquity/controllers/refresh.py +++ b/subiquity/controllers/refresh.py @@ -222,7 +222,7 @@ class RefreshController(SubiquityTuiController): result = await self.app.snapd.get('v2/changes/{}'.format(change)) return result['result'] - def start_ui(self, index=1): + def make_ui(self, index=1): from subiquity.ui.views.refresh import RefreshView if self.app.updated: raise Skip() @@ -239,7 +239,7 @@ class RefreshController(SubiquityTuiController): else: raise AssertionError("unexpected index {}".format(index)) if show: - self.ui.set_body(RefreshView(self)) + return RefreshView(self) else: raise Skip() diff --git a/subiquity/controllers/snaplist.py b/subiquity/controllers/snaplist.py index 0688f9e3..9fe1a19f 100644 --- a/subiquity/controllers/snaplist.py +++ b/subiquity/controllers/snaplist.py @@ -155,13 +155,13 @@ class SnapListController(SubiquityTuiController): self.loader = self._make_loader() self.loader.start() - def start_ui(self): + def make_ui(self): if self.loader.failed or not self.app.base_model.network.has_network: # If loading snaps failed or the network is disabled, skip the # screen. self.configured() raise Skip() - self.ui.set_body(SnapListView(self.model, self)) + return SnapListView(self.model, self) def run_answers(self): if 'snaps' in self.answers: diff --git a/subiquity/controllers/ssh.py b/subiquity/controllers/ssh.py index a625940a..a10d97bc 100644 --- a/subiquity/controllers/ssh.py +++ b/subiquity/controllers/ssh.py @@ -65,8 +65,8 @@ class SSHController(SubiquityTuiController): self.model.pwauth = data.get( 'allow-pw', not self.model.authorized_keys) - def start_ui(self): - self.ui.set_body(SSHView(self.model, self)) + def make_ui(self): + return SSHView(self.model, self) def run_answers(self): if 'ssh-import-id' in self.answers: diff --git a/subiquity/controllers/welcome.py b/subiquity/controllers/welcome.py index 2fbaeb28..979a5379 100644 --- a/subiquity/controllers/welcome.py +++ b/subiquity/controllers/welcome.py @@ -48,9 +48,8 @@ class WelcomeController(SubiquityTuiController): else: self.model.selected_language = lang - def start_ui(self): - view = WelcomeView(self.model, self) - self.ui.set_body(view) + def make_ui(self): + return WelcomeView(self.model, self) def run_answers(self): if 'lang' in self.answers: diff --git a/subiquity/controllers/zdev.py b/subiquity/controllers/zdev.py index 9d3a500c..3b70e438 100644 --- a/subiquity/controllers/zdev.py +++ b/subiquity/controllers/zdev.py @@ -644,7 +644,7 @@ class ZdevController(SubiquityTuiController): zdevinfos = [ZdevInfo.from_row(row) for row in devices] self.zdevinfos = OrderedDict([(i.id, i) for i in zdevinfos]) - def start_ui(self): + def make_ui(self): self.ui.set_body(ZdevView(self)) def run_answers(self): diff --git a/subiquity/ui/views/filesystem/filesystem.py b/subiquity/ui/views/filesystem/filesystem.py index b3cc1103..b4d6fcbf 100644 --- a/subiquity/ui/views/filesystem/filesystem.py +++ b/subiquity/ui/views/filesystem/filesystem.py @@ -535,7 +535,7 @@ class FilesystemView(BaseView): self.show_stretchy_overlay(VolGroupStretchy(self)) def cancel(self, button=None): - self.controller.start_ui() + self.controller.guided() def reset(self, button): self.controller.reset() diff --git a/subiquitycore/controllers/network.py b/subiquitycore/controllers/network.py index 2c8640a4..681edb81 100644 --- a/subiquitycore/controllers/network.py +++ b/subiquitycore/controllers/network.py @@ -449,7 +449,7 @@ class NetworkController(TuiController): dev.set_dhcp_state(v, DHCPState.TIMED_OUT) self.network_event_receiver.update_link(dev.ifindex) - def start_ui(self): + def make_ui(self): if not self.view_shown: self.update_initial_configs() netdev_infos = [ @@ -461,7 +461,7 @@ class NetworkController(TuiController): self.view_shown = True self.view.update_default_routes( self.network_event_receiver.default_routes) - self.ui.set_body(self.view) + return self.view def end_ui(self): self.view = None diff --git a/subiquitycore/core.py b/subiquitycore/core.py index 3f3389f0..e84ade70 100644 --- a/subiquitycore/core.py +++ b/subiquitycore/core.py @@ -41,7 +41,7 @@ class Application: # "InstallProgress", # ] # The 'next_screen' and 'prev_screen' methods move through the list of - # controllers in order, calling the start_ui method on the controller + # controllers in order, calling the make_ui method on the controller # instance. def __init__(self, opts): diff --git a/subiquitycore/tui.py b/subiquitycore/tui.py index 7a8c4ce1..1f2b7332 100644 --- a/subiquitycore/tui.py +++ b/subiquitycore/tui.py @@ -101,7 +101,7 @@ class TuiApplication(Application): if self.opts.screens and new.name not in self.opts.screens: raise Skip try: - new.start_ui() + self.ui.set_body(new.make_ui()) self.cur_screen = new except Skip: new.context.exit("(skipped)") diff --git a/subiquitycore/tuicontroller.py b/subiquitycore/tuicontroller.py index 53d1c36a..40b59f60 100644 --- a/subiquitycore/tuicontroller.py +++ b/subiquitycore/tuicontroller.py @@ -22,7 +22,7 @@ log = logging.getLogger("subiquitycore.tuicontroller") class Skip(Exception): - """Raise this from a controller's start_ui method to skip a screen.""" + """Raise this from a controller's make_ui method to skip a screen.""" class TuiController(BaseController): @@ -45,11 +45,8 @@ class TuiController(BaseController): return inst is self @abstractmethod - def start_ui(self): - """Start running this controller's UI. - - This method should call self.ui.set_body. - """ + def make_ui(self): + """Return the view for this controller's UI.""" def end_ui(self): """Stop running this controller's UI. @@ -109,8 +106,8 @@ class RepeatedController(BaseController): def register_signals(self): pass - def start_ui(self): - self.orig.start_ui(self.index) + def make_ui(self): + return self.orig.make_ui(self.index) def run_answers(self): self.orig.run_answers()