diff --git a/EasyReflectometryApp/Backends/Mock/Project.qml b/EasyReflectometryApp/Backends/Mock/Project.qml index 74825414..680568d2 100644 --- a/EasyReflectometryApp/Backends/Mock/Project.qml +++ b/EasyReflectometryApp/Backends/Mock/Project.qml @@ -6,33 +6,50 @@ QtObject { property bool created: false property string creationDate: '' + property string lastSaved: '' + property bool hasUnsavedChanges: false + + // Like the Python backend, carries the path of the project file that was written. + signal projectSaved(string path) + signal projectSaveError(string message) + function projectFilePath() { return `${location}/${name}/project.json` } property string name: 'Super duper project' - function setName(value) { name = value } + function setName(value) { name = value; hasUnsavedChanges = true } property string description: 'Default project description from Mock proxy' - function setDescription(value) { description = value } + function setDescription(value) { description = value; hasUnsavedChanges = true } property string location: '/path to the project' - function setLocation(value) { location = value } + function setLocation(value) { location = value; hasUnsavedChanges = true } function create() { console.debug(`Creating project ${name}`) creationDate = `${new Date().toLocaleDateString()} ${new Date().toLocaleTimeString()}` created = true + lastSaved = new Date().toISOString() + hasUnsavedChanges = false + projectSaved(projectFilePath()) } function save() { console.debug(`Saving project ${name}`) + lastSaved = new Date().toISOString() + hasUnsavedChanges = false + projectSaved(projectFilePath()) } function reset() { console.debug(`Reset project ${name}`) created = false + lastSaved = '' + hasUnsavedChanges = false } function load(path) { console.debug(`Loading project from ${path}`) creationDate = `${new Date().toLocaleDateString()} ${new Date().toLocaleTimeString()}` created = true + lastSaved = '' + hasUnsavedChanges = false } } diff --git a/EasyReflectometryApp/Backends/Py/logic/material.py b/EasyReflectometryApp/Backends/Py/logic/material.py index 474c1672..0412253e 100644 --- a/EasyReflectometryApp/Backends/Py/logic/material.py +++ b/EasyReflectometryApp/Backends/Py/logic/material.py @@ -3,6 +3,7 @@ from easyreflectometry import Project as ProjectLib from easyreflectometry.sample import MaterialCollection +from easyreflectometry.sample import MaterialDensity logger = logging.getLogger(__name__) @@ -61,7 +62,14 @@ def remove_at_index(self, value: str) -> None: self._materials.pop(int(value)) def add_new(self) -> None: - self._materials.add_material() + # A material added from the GUI is a density material, so that the Material editor's + # detail panel (formula, density, SLD coupling) applies to it: that panel is shown only + # for `kind == 'density'`, and before this the GUI could not produce such a material at + # all — they arrived only through an ORSO sample load. Nothing is lost by the default: + # unchecking the coupling hands the SLD back for direct entry and fitting, which is what + # a plain `Material` offers. The library's defaults (Si at 2.33 g/cm3) also start the + # material at a physical SLD rather than the zero a plain `Material` would carry. + self._materials.add_material(MaterialDensity(name='Material added')) def duplicate_selected(self) -> None: self._materials.duplicate_material(self.index) diff --git a/EasyReflectometryApp/Backends/Py/logic/models.py b/EasyReflectometryApp/Backends/Py/logic/models.py index b5e22e2d..76aaeab7 100644 --- a/EasyReflectometryApp/Backends/Py/logic/models.py +++ b/EasyReflectometryApp/Backends/Py/logic/models.py @@ -117,14 +117,26 @@ def default_model_content(self, model: Model) -> None: def add_new(self) -> None: self._models.add_model() self.default_model_content(self._models[-1]) + self._attach_calculator(self._models[-1]) # Update index to point to the new model self.index = len(self._models) - 1 def duplicate_selected_model(self) -> None: self._models.duplicate_model(self.index) + self._attach_calculator(self._models[-1]) # Update index to point to the duplicated model self.index = len(self._models) - 1 + def _attach_calculator(self, model: Model) -> None: + """Bind a model added through the collection to the project's calculator. + + The collection's `add_model`/`duplicate_model` do not know the project's calculator, so + a model added through them has no interface. The project's fitter is built lazily for + the current model, and `Project.as_dict` (hence every save) touches it, so saving with + such a model selected failed with an internal error instead of writing the file. + """ + model.interface = self._project_lib._calculator + def move_selected_up(self) -> None: if self.index > 0: self._models.move_up(self.index) diff --git a/EasyReflectometryApp/Backends/Py/logic/project.py b/EasyReflectometryApp/Backends/Py/logic/project.py index 32b6a883..57fe9769 100644 --- a/EasyReflectometryApp/Backends/Py/logic/project.py +++ b/EasyReflectometryApp/Backends/Py/logic/project.py @@ -1,3 +1,5 @@ +import hashlib +import json from copy import copy from pathlib import Path @@ -20,6 +22,11 @@ def created(self) -> bool: def path(self) -> str: return str(self._project_lib.path) + @property + def path_json(self) -> str: + """Path of the project file itself, as named in save feedback and error messages.""" + return str(self._project_lib.path_json) + @property def root_path(self) -> str: return str(self._project_lib.path.parent) @@ -118,6 +125,20 @@ def info(self) -> dict: info['location'] = self._project_lib.path return info + def content_fingerprint(self) -> str: + """A digest of exactly what `save()` would write. + + Used to tell a real edit from a signal that merely looks like one: selecting another + model or assembly emits the same signals as editing them, but does not change this. + Costs one serialization (under 10 ms with an experiment loaded), so callers keep it to + the moment a project might turn from clean to edited, not to every signal. + + :raises Exception: whatever `as_dict` raises; a caller that cannot fingerprint the + project must treat it as changed. + """ + content = self._project_lib.as_dict(include_materials_not_in_model=True) + return hashlib.sha256(json.dumps(content, sort_keys=True).encode('utf-8')).hexdigest() + def create(self) -> None: self._project_lib.create() self._project_lib.save_as_json() diff --git a/EasyReflectometryApp/Backends/Py/project.py b/EasyReflectometryApp/Backends/Py/project.py index 50337442..bdc6b036 100644 --- a/EasyReflectometryApp/Backends/Py/project.py +++ b/EasyReflectometryApp/Backends/Py/project.py @@ -1,4 +1,7 @@ +import logging import warnings +from contextlib import contextmanager +from datetime import datetime from easyreflectometry import Project as ProjectLib from easyreflectometry.orso_utils import load_orso_model @@ -11,12 +14,16 @@ from .helpers import IO from .logic.project import Project as ProjectLogic +logger = logging.getLogger(__name__) + class Project(QObject): createdChanged = Signal() nameChanged = Signal() descriptionChanged = Signal() locationChanged = Signal() + lastSavedChanged = Signal() + hasUnsavedChangesChanged = Signal() externalCreatedChanged = Signal() externalNameChanged = Signal() @@ -24,10 +31,19 @@ class Project(QObject): externalProjectReset = Signal() sampleLoadWarning = Signal(str) projectLoadError = Signal(str) + projectSaved = Signal(str) + projectSaveError = Signal(str) def __init__(self, project_lib: ProjectLib, parent=None): super().__init__(parent) self._logic = ProjectLogic(project_lib) + self._last_saved = '' + self._has_unsaved_changes = False + self._dirty_suspended = 0 + # Fingerprint of the project content as it was last known to match the disk (taken on + # create, save, load and reset). None means "unknown", in which case a dirtying signal + # is trusted as is. + self._clean_fingerprint = None # Properties @@ -43,6 +59,27 @@ def creationDate(self) -> str: def currentProjectPath(self) -> str: return self._logic.path + @Property(str, notify=lastSavedChanged) + def lastSaved(self) -> str: + """ISO-8601 time of the last successful save, with UTC offset, or '' if never saved. + + This is the time of the last save made from this session, which is deliberately not + the same as `creationDate` (the project's stored modification stamp). The value is + left unformatted so that QML can render it with a locale-aware `Qt.formatTime`. + """ + return self._last_saved + + @Property(bool, notify=hasUnsavedChangesChanged) + def hasUnsavedChanges(self) -> bool: + """Whether the project holds edits that `save()` would write to disk. + + Set from every backend signal that changes what the project file would contain (the + inventory lives in `py_backend.DIRTYING_SIGNALS`) and from this object's own setters; + cleared by create, save, load and reset. Always False while no project has been + created, since there is nothing on disk for the edits to differ from. + """ + return self._has_unsaved_changes + # Properties with setters @Property(str, notify=nameChanged) @@ -53,6 +90,7 @@ def name(self) -> str: def setName(self, new_value: str) -> None: if self._logic.name != new_value: self._logic.name = new_value + self.markDirty() self.nameChanged.emit() self.externalNameChanged.emit() @@ -64,6 +102,7 @@ def description(self) -> str: def setDescription(self, new_value: str) -> None: if self._logic.description != new_value: self._logic.description = new_value + self.markDirty() self.descriptionChanged.emit() @Property(str, notify=locationChanged) @@ -74,20 +113,155 @@ def location(self) -> str: def setLocation(self, new_value: str) -> None: if self._logic.root_path != new_value: self._logic.root_path = new_value + self.markDirty() self.locationChanged.emit() # Methods + @Slot() + def markDirty(self) -> None: + """Record that the project differs from the file on disk. + + Connected to every dirtying backend signal by `py_backend._connect_dirty_tracking`, and + called directly by this object's setters. + + Ignored while no project has been created (nothing on disk to differ from, and `save()` + refuses anyway) and while a create/load/reset is fanning out its own signals. The + signals are a fast, over-approximate trigger: some fire on a mere selection change, and + the sample's coalesced `constraintsChanged` fires one event-loop turn after the load + that caused it. So the clean-to-edited transition is confirmed against the content + fingerprint recorded at the last clean point, which costs one serialization per + transition rather than one per signal. + """ + if self._dirty_suspended or self._has_unsaved_changes: + return + if not self._logic.created: + return + if self._content_unchanged_since_clean(): + return + self._set_dirty() + + def _content_unchanged_since_clean(self) -> bool: + if self._clean_fingerprint is None: + return False + try: + return self._logic.content_fingerprint() == self._clean_fingerprint + except Exception: + # A project that cannot be serialized right now cannot be proven unchanged; the + # save path will report the actual problem. + logger.debug('Could not fingerprint the project; treating it as changed', exc_info=True) + return False + + def _record_clean_state(self) -> None: + """Remember the current content as matching the disk and clear the flag.""" + try: + self._clean_fingerprint = self._logic.content_fingerprint() + except Exception: + logger.debug('Could not fingerprint the project after a clean point', exc_info=True) + self._clean_fingerprint = None + self._clear_dirty() + + def _set_dirty(self) -> None: + if self._has_unsaved_changes: + return + self._has_unsaved_changes = True + self.hasUnsavedChangesChanged.emit() + + def _clear_dirty(self) -> None: + if not self._has_unsaved_changes: + return + self._has_unsaved_changes = False + self.hasUnsavedChangesChanged.emit() + + @contextmanager + def _suspended_dirty_tracking(self): + """Run a create/load/reset without its own relays marking the project dirty. + + Those relays run through the sample, experiment and analysis parts, which emit the very + signals dirty tracking listens to. Suppressing them during the fan-out keeps the + fan-out from fingerprinting the project once per signal. (Deferred emissions land after + the block and are caught by the fingerprint check instead.) + + Suspending is all this does; clearing the flag is left to the callers, because only they + know whether the change actually reached disk. + """ + self._dirty_suspended += 1 + try: + yield + finally: + self._dirty_suspended -= 1 + + def _clear_last_saved(self) -> None: + """Drop the save stamp when the project it referred to is gone (reset or load).""" + if self._last_saved: + self._last_saved = '' + self.lastSavedChanged.emit() + + def _mark_saved(self) -> None: + self._record_clean_state() + # Aware stamp: unambiguous if it is ever logged or shown outside the local session. + self._last_saved = datetime.now().astimezone().isoformat(timespec='seconds') + self.lastSavedChanged.emit() + self.projectSaved.emit(self._logic.path_json) + + def _save_error_message(self, exception: Exception) -> str: + """Turn a save failure into a sentence a user can act on, keeping the raw text as detail. + + The library raises rather than prints, so these are the failures that actually reach the + GUI. The previously saved file is always intact. + """ + path = self._logic.path_json + if isinstance(exception, FileExistsError): + # Only create() can raise this (save() overwrites): the project directory is taken. + explanation = f'A project already exists at "{self._logic.path}".\nChoose a different name or location.' + elif isinstance(exception, PermissionError): + explanation = f'No permission to write "{path}".\nThe file may be read-only or open in another program.' + elif isinstance(exception, (TypeError, ValueError)): + # Raised while serializing, e.g. a constraint that depends on a parameter which is + # not reachable from the models. + explanation = f'The project could not be saved to "{path}" because it cannot be serialized.' + elif isinstance(exception, OSError): + explanation = f'The project could not be written to "{path}".' + else: + return f'Failed to save the project to "{path}".\n\n{exception}' + return f'{explanation}\n\nDetails: {exception}' + @Slot() def create(self) -> None: - self._logic.create() - self.createdChanged.emit() - self.externalCreatedChanged.emit() + # create() writes the project file, so it is a first save and reports through the same + # signals. It can fail on a colliding path, which the library raises instead of printing. + error = None + with self._suspended_dirty_tracking(): + try: + self._logic.create() + except Exception as ex: + error = self._save_error_message(ex) + # Emitted either way, so that the UI reflects the real `created` state even when the + # directories were made but the file was not written. + self.createdChanged.emit() + self.externalCreatedChanged.emit() + if error is not None: + if self._logic.created: + # The library makes the directories before writing the file, so a failure of the + # write alone leaves a project the UI considers created with nothing of it on + # disk. That state has to be dirty: the Save button, Ctrl+S and the close prompt + # are all gated on the flag, and without it the only way to retry the write is to + # make an unrelated edit first — while closing the window discards the work + # without asking. + self._clean_fingerprint = None + self._set_dirty() + self.projectSaveError.emit(error) + else: + self._mark_saved() @Slot(str) def load(self, path: str) -> None: + path = IO.generalizePath(path) try: - self._logic.load(IO.generalizePath(path)) + self._logic.load(path) + except FileNotFoundError: + self.projectLoadError.emit(f'The project file "{path}" does not exist.') + return except ValueError as ex: # easyreflectometry rejects project files whose file_format predates # the current schema. Show a user-facing message for that case and @@ -102,26 +276,51 @@ def load(self, path: str) -> None: message = str(ex) self.projectLoadError.emit(message) return - self.createdChanged.emit() - self.nameChanged.emit() - self.descriptionChanged.emit() - self.locationChanged.emit() - self.externalProjectLoaded.emit() + except OSError as ex: + self.projectLoadError.emit(f'The project file "{path}" could not be read.\n\nDetails: {ex}') + return + self._clear_last_saved() + # The fingerprint is taken before the fan-out: whatever the relays emit, now or on a + # later event-loop turn, is compared against the state that was just loaded. + self._record_clean_state() + with self._suspended_dirty_tracking(): + self.createdChanged.emit() + self.nameChanged.emit() + self.descriptionChanged.emit() + self.locationChanged.emit() + self.externalProjectLoaded.emit() + self._clear_dirty() @Slot() def save(self) -> None: - self._logic.save() + if not self._logic.created: + # Nothing has been created, so there is no project file of our own to update; saving + # would write the in-memory defaults over whatever sits at the current path. + self.projectSaveError.emit('No project has been created yet.\nCreate or open a project before saving.') + return + # The whole call is guarded: the library's unlink-free atomic save raises, and a locked + # destination raises out of os.replace, so nothing may escape into this slot uncaught. + try: + self._logic.save() + except Exception as ex: + self.projectSaveError.emit(self._save_error_message(ex)) + return + self._mark_saved() @Slot() def reset(self) -> None: self._logic.reset() - self.createdChanged.emit() - self.nameChanged.emit() - self.descriptionChanged.emit() - self.locationChanged.emit() - self.externalCreatedChanged.emit() - self.externalNameChanged.emit() - self.externalProjectReset.emit() + self._clear_last_saved() + self._record_clean_state() + with self._suspended_dirty_tracking(): + self.createdChanged.emit() + self.nameChanged.emit() + self.descriptionChanged.emit() + self.locationChanged.emit() + self.externalCreatedChanged.emit() + self.externalNameChanged.emit() + self.externalProjectReset.emit() + self._clear_dirty() @Slot(str, bool) def sampleLoad(self, url: str, append: bool = True) -> None: @@ -146,5 +345,8 @@ def sampleLoad(self, url: str, append: bool = True) -> None: else: # Replace all existing models with the loaded sample self._logic.replace_models_from_orso(sample) + # An imported sample is project content, unlike the project loads this signal otherwise + # announces; marked here rather than left to the relay's incidental table signals. + self.markDirty() # notify listeners self.externalProjectLoaded.emit() diff --git a/EasyReflectometryApp/Backends/Py/py_backend.py b/EasyReflectometryApp/Backends/Py/py_backend.py index 59a6f8a2..731133a8 100644 --- a/EasyReflectometryApp/Backends/Py/py_backend.py +++ b/EasyReflectometryApp/Backends/Py/py_backend.py @@ -15,6 +15,56 @@ from .status import Status from .summary import Summary +# Signals whose emission changes what `Project.save()` would write to disk. Every one of them +# marks the project dirty (see `_connect_dirty_tracking`). The inventory is explicit rather than +# derived from a naming convention because the `external*` relays do not cover every mutation: +# a material rename emits only `materialsTableChanged`, reordering layers or editing a repeated +# assembly's repetitions emits only `externalRefreshPlot`, and the free/fixed checkbox emits only +# `parametersChanged`. Missing one of those means the close prompt does not fire and the user +# loses work, so anything doubtful belongs in this list rather than out of it. +# +# `Project`'s own signals are absent on purpose: its setters call `markDirty` directly, because +# create, load and reset fan out through those same signal names and must end up clean. +DIRTYING_SIGNALS = { + '_sample': ( + 'externalSampleChanged', + 'externalRefreshPlot', + 'materialsTableChanged', + 'modelsTableChanged', + 'constraintsChanged', + 'calculationEngineChanged', # stored as 'calculator' in the project file + 'qRangeChanged', + ), + '_experiment': ( + 'externalExperimentChanged', + 'experimentLoaded', + 'qRangeUpdated', + ), + '_analysis': ( + 'externalCalculatorChanged', + 'externalExperimentChanged', + 'externalFittingChanged', # a finished fit rewrites parameter values + 'externalMinimizerChanged', # stored as 'fitter_minimizer' in the project file + 'externalParametersChanged', + 'parametersChanged', + ), +} + +# `external*` signals deliberately left out of dirty tracking, with the reason. Together with +# DIRTYING_SIGNALS this must account for every `external*` signal on every backend part; +# tests/test_py_backend.py fails if a new one appears in neither, so it cannot silently skip +# dirty tracking. +NON_DIRTYING_EXTERNAL_SIGNALS = { + '_project': ( + # Project lifecycle, not project content: create/load/reset end in a clean project and + # the setters mark dirty themselves. + 'externalCreatedChanged', + 'externalNameChanged', + 'externalProjectLoaded', + 'externalProjectReset', + ), +} + class PyBackend(QObject): # Signal for multi-experiment selection changes @@ -220,6 +270,14 @@ def _connect_backend_parts(self) -> None: self._connect_sample_page() self._connect_experiment_page() self._connect_analysis_page() + self._connect_dirty_tracking() + + def _connect_dirty_tracking(self) -> None: + """Route every content-changing signal to the project's unsaved-changes flag.""" + for part_name, signal_names in DIRTYING_SIGNALS.items(): + part = getattr(self, part_name) + for signal_name in signal_names: + getattr(part, signal_name).connect(self._project.markDirty) ######### Forming connections between the backend parts def _connect_project_page(self) -> None: diff --git a/EasyReflectometryApp/Backends/Py/sample.py b/EasyReflectometryApp/Backends/Py/sample.py index 1281bb30..4f432219 100644 --- a/EasyReflectometryApp/Backends/Py/sample.py +++ b/EasyReflectometryApp/Backends/Py/sample.py @@ -533,15 +533,20 @@ def setCurrentLayerIndex(self, new_value: int) -> None: self._project_lib.current_layer_index = new_value self.layersIndexChanged.emit() + # A rename is project content and shows up in the Analysis parameter names, so it emits + # externalSampleChanged like the assembly rename does; layersChange alone would only reach + # dirty tracking through the deferred constraintsChanged relay, by accident. @Slot(str) def setCurrentLayerName(self, new_value: str) -> None: if self._layers_logic.set_name_at_current_index(new_value): self._clearCacheAndEmitLayersChanged() + self.externalSampleChanged.emit() @Slot(int, str) def setLayerNameAtIndex(self, index: int, new_value: str) -> None: if self._layers_logic.set_name_at_index(index, new_value): self._clearCacheAndEmitLayersChanged() + self.externalSampleChanged.emit() @Slot(int) def setCurrentLayerMaterial(self, new_value: int) -> None: diff --git a/EasyReflectometryApp/Gui/ApplicationWindow.qml b/EasyReflectometryApp/Gui/ApplicationWindow.qml index 13777efa..8987f0cb 100644 --- a/EasyReflectometryApp/Gui/ApplicationWindow.qml +++ b/EasyReflectometryApp/Gui/ApplicationWindow.qml @@ -30,11 +30,41 @@ EaComponents.ApplicationWindow { appBarLeftButtons: [ EaElements.ToolButton { - enabled: Globals.BackendWrapper.projectCreated + id: saveButton + // Disabled when the project is already on disk unchanged, so that an enabled button + // is itself the signal that there is something to save. Saving also serializes the + // same model state the fitter thread is writing to, so it is blocked during a fit + // rather than silently storing half-updated parameters. The success flash keeps the + // enabled style, otherwise the check mark is drawn greyed out because the save that + // triggered it has just disabled the button. + enabled: applicationWindow.canSaveProject || saveFlashTimer.running highlighted: true - fontIcon: "save" - ToolTip.text: qsTr("Save current state of the project") - onClicked: Globals.BackendWrapper.projectSave() + fontIcon: saveFlashTimer.running ? "check-circle" : "save" + ToolTip.text: { + if (saveFlashTimer.running) { + return qsTr("Project saved") + } + if (Globals.BackendWrapper.analysisFittingRunning) { + return qsTr("Saving is disabled while a fit is running") + } + if (Globals.BackendWrapper.projectCreated && !Globals.BackendWrapper.projectHasUnsavedChanges) { + return qsTr("No changes to save") + } + return qsTr("Save current state of the project") + } + onClicked: { + if (applicationWindow.canSaveProject) { + Globals.BackendWrapper.projectSave() + } + } + + // Success feedback in place, where the user just clicked. A save during the flash + // restarts it rather than cutting it short. + Timer { + id: saveFlashTimer + interval: 2000 + repeat: false + } }, EaElements.ToolButton { @@ -164,16 +194,142 @@ EaComponents.ApplicationWindow { // MISC /////// - onClosing: Qt.quit() + // Whether there is a project on disk that differs from what is in memory and can be written + // right now. Shared by the Save button, the Ctrl+S shortcut and the close prompt. + readonly property bool canSaveProject: Globals.BackendWrapper.projectCreated + && Globals.BackendWrapper.projectHasUnsavedChanges + && !Globals.BackendWrapper.analysisFittingRunning + + // Closing with unsaved work asks first. Qt5 had this (Components/CloseDialog.qml); the Qt6 + // migration replaced it with `onClosing: Qt.quit()`, which exited unconditionally without + // asking. This restores the prompt, with the Cancel button the Qt5 dialog was missing. + // + // The prompt is gated on a created project as well as on the flag: before a create there is + // nothing on disk that the edits could be "unsaved" relative to, and "Save and exit" would + // otherwise write the defaults over whatever project sits at the chosen path. + onClosing: function(close) { + if (Globals.BackendWrapper.projectCreated + && Globals.BackendWrapper.projectHasUnsavedChanges + && !applicationWindow.discardChangesOnClose) { + close.accepted = false + closeDialog.open() + } + } + + // Set by "Exit without saving" (and test mode) before the window is closed. In Qt 6, + // Qt.quit() does not stop the event loop directly: it asks every top-level window to close + // first, which runs onClosing again, so a handler that rejects the close while the project + // is dirty would swallow the quit and reopen the prompt forever. Recording the decision here + // lets onClosing honour it. + property bool discardChangesOnClose: false + + // Set while the close dialog's "Save and exit" is in flight. Unlike Qt5, which quit + // unconditionally, the app only exits once the save has actually reported success — a failed + // save leaves the window open with its error dialog rather than discarding the work. + property bool quitAfterSave: false + + EaElements.Dialog { + id: closeDialog + title: qsTr('Unsaved Changes') + // Modal, so the user cannot keep editing, start a fit or hit the window's close button + // again behind the question. (EaElements.Dialog is modeless by default.) + modal: true + closePolicy: Popup.CloseOnEscape + + EaElements.Label { + text: qsTr('The project has unsaved changes.\nDo you want to save them before exiting?') + wrapMode: Text.WordWrap + width: EaStyle.Sizes.sideBarContentWidth + } + + footer: EaElements.DialogButtonBox { + EaElements.Button { + text: qsTr('Cancel') + onClicked: closeDialog.close() + } + + EaElements.Button { + text: qsTr('Exit without saving') + onClicked: { + closeDialog.close() + applicationWindow.discardChangesOnClose = true + applicationWindow.close() + } + } + + EaElements.Button { + text: qsTr('Save and exit') + enabled: applicationWindow.canSaveProject + onClicked: { + closeDialog.close() + applicationWindow.quitAfterSave = true + Globals.BackendWrapper.projectSave() + } + } + } + } + + Shortcut { + sequences: [StandardKey.Save] + enabled: applicationWindow.canSaveProject + onActivated: Globals.BackendWrapper.projectSave() + } + + // Save feedback is asymmetric: a failure must not be missable, so it is modal, while a + // successful save flashes the tool button and updates the status bar instead of interrupting. + Connections { + target: Globals.BackendWrapper + ignoreUnknownSignals: true + + function onProjectSaved(path) { + saveFlashTimer.restart() + if (applicationWindow.quitAfterSave) { + applicationWindow.quitAfterSave = false + Qt.quit() + } + } + + function onProjectSaveError(message) { + // A failed "Save and exit" must not exit: the work is still only in memory. + applicationWindow.quitAfterSave = false + projectSaveErrorDialog.errorMessage = message + projectSaveErrorDialog.open() + } + } + + EaElements.Dialog { + id: projectSaveErrorDialog + title: qsTr('Project Save Error') + standardButtons: Dialog.Ok + modal: true + closePolicy: Popup.CloseOnEscape | Popup.CloseOnPressOutside + + property string errorMessage: '' + + EaElements.Label { + text: projectSaveErrorDialog.errorMessage + wrapMode: Text.WordWrap + width: EaStyle.Sizes.sideBarContentWidth + } + } EaElements.Dialog { id: resetStateDialog title: qsTr("Reset state") + modal: true EaElements.Label { horizontalAlignment: Text.AlignHCenter - text: qsTr("Are you sure you want to reset the application to its\noriginal state without project, sample and data?\n\nThis operation cannot be undone.") + // Now that unsaved changes are tracked, the dialog can say what is actually at risk + // instead of warning about loss that may not exist. + text: { + const question = qsTr("Are you sure you want to reset the application to its\noriginal state without project, sample and data?") + if (Globals.BackendWrapper.projectHasUnsavedChanges) { + return question + '\n\n' + qsTr("The project has unsaved changes that will be lost.") + } + return question + '\n\n' + qsTr("This operation cannot be undone.") + } } footer: EaElements.DialogButtonBox { @@ -249,6 +405,8 @@ EaComponents.ApplicationWindow { console.debug('*** TEST MODE START ***') delay(30000, function() { console.debug('*** TEST MODE 30 s DELAYED END ***') + // The harness must never hang on the unsaved-changes prompt. + applicationWindow.discardChangesOnClose = true Qt.quit() }) } diff --git a/EasyReflectometryApp/Gui/Globals/BackendWrapper.qml b/EasyReflectometryApp/Gui/Globals/BackendWrapper.qml index 426ceaae..68c08b2c 100644 --- a/EasyReflectometryApp/Gui/Globals/BackendWrapper.qml +++ b/EasyReflectometryApp/Gui/Globals/BackendWrapper.qml @@ -100,6 +100,28 @@ QtObject { return null } + // Project save signals - forwarded from backend + readonly property string projectLastSaved: activeBackend.project.lastSaved ?? '' + readonly property bool projectHasUnsavedChanges: activeBackend.project.hasUnsavedChanges ?? false + + signal projectSaved(string path) + + property var _projectSavedConnection: { + if (activeBackend && activeBackend.project && activeBackend.project.projectSaved) { + activeBackend.project.projectSaved.connect(projectSaved) + } + return null + } + + signal projectSaveError(string message) + + property var _projectSaveErrorConnection: { + if (activeBackend && activeBackend.project && activeBackend.project.projectSaveError) { + activeBackend.project.projectSaveError.connect(projectSaveError) + } + return null + } + /////////////// // Sample page diff --git a/EasyReflectometryApp/Gui/StatusBar.qml b/EasyReflectometryApp/Gui/StatusBar.qml index 6b14a296..c1590ef5 100644 --- a/EasyReflectometryApp/Gui/StatusBar.qml +++ b/EasyReflectometryApp/Gui/StatusBar.qml @@ -54,6 +54,23 @@ EaElements.StatusBar { ToolTip.text: qsTr('Number of parameters: total, free and fixed') } + EaElements.StatusBarItem { + visible: Globals.BackendWrapper.projectLastSaved !== '' + keyIcon: 'save' + keyText: qsTr('Saved') + // The backend reports an ISO-8601 stamp so that the time is rendered in the user's locale. + // Minute resolution: some locales' short time format carries seconds, which is more + // precision than a save stamp needs. + valueText: { + if (Globals.BackendWrapper.projectLastSaved === '') { + return '' + } + const format = Qt.locale().timeFormat(Locale.ShortFormat).replace(/[:.]?s+/g, '') + return Qt.formatTime(new Date(Globals.BackendWrapper.projectLastSaved), format) + } + ToolTip.text: qsTr('Time of the last successful save') + } + EaElements.StatusBarItem { visible: Globals.BackendWrapper.analysisFittingRunning keyIcon: 'play-circle' diff --git a/tests/factories.py b/tests/factories.py index 3863d1ff..dd022373 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -378,6 +378,7 @@ def __init__( self._info = {'name': 'Demo Project', 'short_description': 'Demo Description', 'modified': '2026-03-19'} self.created = False self.path = Path('C:/tmp/demo-project') + self.path_json = self.path / 'project.json' self.q_min = 0.01 self.q_max = 0.5 self.q_resolution = 200 diff --git a/tests/test_logic_material.py b/tests/test_logic_material.py index cf83ba44..936f7408 100644 --- a/tests/test_logic_material.py +++ b/tests/test_logic_material.py @@ -113,6 +113,29 @@ def test_set_density_at_index_clamps_below_the_min_bound(): assert materials[0].density.value == pytest.approx(0.0) +def test_material_logic_add_new_creates_a_density_material(): + """A material added from the GUI must be a density material. + + The Material editor shows its formula/density/SLD-coupling panel only for + `kind == 'density'`, so a plain `Material` (what the collection appends by default) + gives the user a row with no way to reach any of that. + """ + materials = make_material_collection(make_material('Air', sld=0.0)) + project = make_project(materials=materials) + logic = Material(project) + + logic.add_new() + + added = logic.materials[-1] + assert added['label'] == 'Material added' + assert added['kind'] == 'density' + # The library's defaults, so the material starts at a physical SLD rather than zero. + assert added['formula'] == 'Si' + assert float(added['density']) == pytest.approx(2.33) + assert float(added['sld']) == pytest.approx(2.0737, abs=1e-4) + assert added['sld_coupled'] is True + + def test_material_logic_add_duplicate_move_and_remove(): materials = make_material_collection( make_material('Air', sld=0.0), diff --git a/tests/test_logic_project.py b/tests/test_logic_project.py index 971e01e0..f9050ec0 100644 --- a/tests/test_logic_project.py +++ b/tests/test_logic_project.py @@ -1,5 +1,7 @@ from pathlib import Path +import pytest + from EasyReflectometryApp.Backends.Py.logic.project import Project from tests.factories import make_assembly from tests.factories import make_layer @@ -111,3 +113,19 @@ def test_project_reset_calls_reset_and_default_model(): assert project_lib.calls == [('reset',), ('default_model',)] + + +def test_save_and_create_propagate_library_failures(): + """The logic layer must not swallow save failures; the backend turns them into a dialog.""" + project_lib = make_project_with_sample() + logic = Project(project_lib) + + def _raise(overwrite=False): + raise OSError('disk full') + + project_lib.save_as_json = _raise + + with pytest.raises(OSError): + logic.save() + with pytest.raises(OSError): + logic.create() diff --git a/tests/test_py_backend.py b/tests/test_py_backend.py index baf962bf..1b1e8310 100644 --- a/tests/test_py_backend.py +++ b/tests/test_py_backend.py @@ -24,6 +24,10 @@ class StubProject(QObject): def __init__(self, _project_lib, parent=None): super().__init__(parent) + self.dirty_calls = 0 + + def markDirty(self): + self.dirty_calls += 1 class StubSample(QObject): diff --git a/tests/test_py_dirty_tracking.py b/tests/test_py_dirty_tracking.py new file mode 100644 index 00000000..49a04984 --- /dev/null +++ b/tests/test_py_dirty_tracking.py @@ -0,0 +1,302 @@ +"""Dirty tracking against the real backend. + +Two layers are under test. The inventory (`DIRTYING_SIGNALS`): a mutation path whose signal is +missing from it leaves `hasUnsavedChanges` False, so the close prompt never fires and the user's +work is discarded silently; `test_every_external_signal_is_classified` makes that hard to do by +accident. And the content check behind the flag: the signals over-approximate (a selection change +emits the same ones as an edit, and the sample relays `layersChange` into `constraintsChanged` +through a 0 ms timer, so a load's own fan-out lands after the load has finished), so the flag +only flips when the project content actually differs from the last clean state. + +The event-loop tests here are the ones that matter: a synchronous check passes while a freshly +loaded project turns dirty one `processEvents()` later. +""" + +import pytest +from PySide6.QtCore import QMetaMethod +from PySide6.QtCore import Signal + +from EasyReflectometryApp.Backends.Py import py_backend as backend_module +from EasyReflectometryApp.Backends.Py.analysis import Analysis +from EasyReflectometryApp.Backends.Py.experiment import Experiment +from EasyReflectometryApp.Backends.Py.project import Project +from EasyReflectometryApp.Backends.Py.sample import Sample + +PART_CLASSES = { + '_project': Project, + '_sample': Sample, + '_experiment': Experiment, + '_analysis': Analysis, +} + + +def _signal_names(cls) -> set: + return {name for name, value in vars(cls).items() if isinstance(value, Signal)} + + +_ARGUMENT_DEFAULTS = {'int': 0, 'double': 0.0, 'float': 0.0, 'QString': '', 'bool': False} + + +def _emit(owner, signal_name: str) -> None: + """Emit a signal without caring what it carries; only the connection is under test.""" + meta_object = owner.metaObject() + for index in range(meta_object.methodCount()): + method = meta_object.method(index) + if method.methodType() != QMetaMethod.MethodType.Signal: + continue + if bytes(method.name()).decode() != signal_name: + continue + argument_types = [bytes(parameter).decode() for parameter in method.parameterTypes()] + getattr(owner, signal_name).emit(*(_ARGUMENT_DEFAULTS[each] for each in argument_types)) + return + raise AssertionError(f'{type(owner).__name__} exposes no signal {signal_name} to Qt') + + +@pytest.fixture(scope='module') +def _backend(qcore_application): + return backend_module.PyBackend() + + +@pytest.fixture +def py_backend(_backend, qcore_application, tmp_path): + """The shared backend with a freshly created, clean project in a temporary directory.""" + project = _backend._project + project.reset() + # setLocation takes the parent of what it is given; the project ends up at tmp_path/. + project.setLocation(str(tmp_path / 'anything')) + project.setName('DirtyTrackingProject') + project.create() + qcore_application.processEvents() + assert project.created is True + assert project.hasUnsavedChanges is False + return _backend + + +def _forget_clean_state(project) -> None: + """Make the next dirtying signal count regardless of content: the connection is under test.""" + project._clear_dirty() + project._clean_fingerprint = None + + +def test_every_external_signal_is_classified(): + """A new external* signal must be declared as dirtying or explicitly excluded. + + This fails when someone adds one and wires it nowhere, which is the failure mode that costs + a user their unsaved fit. + """ + for part_name, cls in PART_CLASSES.items(): + external = {name for name in _signal_names(cls) if name.startswith('external')} + classified = set(backend_module.DIRTYING_SIGNALS.get(part_name, ())) | set( + backend_module.NON_DIRTYING_EXTERNAL_SIGNALS.get(part_name, ()) + ) + unclassified = external - classified + assert not unclassified, ( + f'{cls.__name__} signal(s) {sorted(unclassified)} are neither in DIRTYING_SIGNALS nor ' + f'in NON_DIRTYING_EXTERNAL_SIGNALS. Decide whether they change what save() writes.' + ) + + +def test_declared_signals_exist_on_their_backend_part(): + """Guards against a rename silently emptying the inventory.""" + for mapping in (backend_module.DIRTYING_SIGNALS, backend_module.NON_DIRTYING_EXTERNAL_SIGNALS): + for part_name, signal_names in mapping.items(): + cls = PART_CLASSES[part_name] + missing = set(signal_names) - _signal_names(cls) + assert not missing, f'{cls.__name__} has no signal(s) {sorted(missing)}' + + +@pytest.mark.parametrize( + ('part_name', 'signal_name'), + [(part, signal) for part, signals in backend_module.DIRTYING_SIGNALS.items() for signal in signals], +) +def test_each_dirtying_signal_is_connected(py_backend, part_name, signal_name): + """Emitting the signal on a real backend must set the flag — proves the connection exists.""" + project = py_backend._project + _forget_clean_state(project) + assert project.hasUnsavedChanges is False + + _emit(getattr(py_backend, part_name), signal_name) + + assert project.hasUnsavedChanges is True, f'{part_name}.{signal_name} does not mark the project dirty' + + +def test_setters_mark_a_created_project_dirty(py_backend): + project = py_backend._project + + project.setName('A new name') + assert project.hasUnsavedChanges is True + + project._record_clean_state() + project.setDescription('A new description') + assert project.hasUnsavedChanges is True + + +def test_setter_that_changes_nothing_does_not_dirty(py_backend): + project = py_backend._project + + project.setName(project.name) + project.setDescription(project.description) + + assert project.hasUnsavedChanges is False + + +# --- Lifecycle: clean now, and still clean once the event loop has turned ----------------------- + + +def test_create_stays_clean_after_the_event_loop_turns(py_backend, qcore_application): + project = py_backend._project + # The fixture created the project and turned the loop once already; turn it again to be sure + # nothing is still pending. + qcore_application.processEvents() + assert project.hasUnsavedChanges is False + + +def test_reset_stays_clean_after_the_event_loop_turns(py_backend, qcore_application): + project = py_backend._project + py_backend._sample.setCurrentMaterialSld(1.234) + assert project.hasUnsavedChanges is True + + project.reset() + assert project.hasUnsavedChanges is False + qcore_application.processEvents() + + assert project.hasUnsavedChanges is False + + +def test_load_stays_clean_after_the_event_loop_turns(py_backend, qcore_application): + project = py_backend._project + path_json = project._logic.path_json + py_backend._sample.setCurrentMaterialSld(1.234) + project.save() + assert project.hasUnsavedChanges is False + + py_backend._sample.setCurrentMaterialSld(5.678) + assert project.hasUnsavedChanges is True + project.load(path_json) + assert project.hasUnsavedChanges is False + qcore_application.processEvents() + + assert project.hasUnsavedChanges is False + assert project.created is True + + +def test_load_of_a_missing_file_reports_and_keeps_the_project(py_backend, qcore_application, tmp_path): + project = py_backend._project + errors = [] + project.projectLoadError.connect(lambda message: errors.append(message)) + + project.load(str(tmp_path / 'nowhere' / 'project.json')) + + assert len(errors) == 1 + assert 'does not exist' in errors[0] + assert project.created is True + assert project.name == 'DirtyTrackingProject' + + +# --- Selection is not content ------------------------------------------------------------------ + + +def test_selecting_a_model_assembly_or_layer_does_not_dirty(py_backend, qcore_application): + project = py_backend._project + sample = py_backend._sample + sample.addNewModel() + assert project.hasUnsavedChanges is True + project.save() + assert project.hasUnsavedChanges is False + + sample.setCurrentModelIndex(1) + sample.setCurrentModelIndex(0) + sample.setCurrentAssemblyIndex(1) + sample.setCurrentAssemblyIndex(0) + sample.setCurrentLayerIndex(0) + qcore_application.processEvents() + + assert project.hasUnsavedChanges is False + + +def test_selecting_an_experiment_does_not_dirty(py_backend, qcore_application): + project = py_backend._project + analysis = py_backend._analysis + + analysis.setExperimentCurrentIndex(0) + analysis.setSelectedExperimentIndices([]) + py_backend.analysisSetSelectedExperimentIndices([0]) + qcore_application.processEvents() + + assert project.hasUnsavedChanges is False + + +# --- Edits are content --------------------------------------------------------------------------- + + +def test_renaming_a_layer_dirties_immediately(py_backend): + """Persisted in the project file, and tracked on its own rather than through the deferred + constraints relay.""" + project = py_backend._project + + py_backend._sample.setCurrentLayerName('Renamed layer') + + assert project.hasUnsavedChanges is True + + +def test_renaming_a_layer_by_index_dirties_immediately(py_backend): + project = py_backend._project + + py_backend._sample.setLayerNameAtIndex(0, 'Renamed layer') + + assert project.hasUnsavedChanges is True + + +@pytest.mark.parametrize( + 'edit', + [ + lambda backend: backend._sample.setCurrentAssemblyName('Renamed assembly'), + lambda backend: backend._sample.setCurrentMaterialSld(1.234), + lambda backend: backend._sample.addNewModel(), + lambda backend: backend._project.setDescription('A new description'), + ], + ids=['assembly rename', 'material sld', 'add model', 'description'], +) +def test_content_edits_dirty(py_backend, edit): + project = py_backend._project + + edit(py_backend) + + assert project.hasUnsavedChanges is True + + +def test_save_records_the_new_clean_state(py_backend, qcore_application): + project = py_backend._project + py_backend._sample.setCurrentMaterialSld(1.234) + project.save() + assert project.hasUnsavedChanges is False + + # The same signals again, without an edit: still clean. + py_backend._sample.setCurrentAssemblyIndex(0) + qcore_application.processEvents() + assert project.hasUnsavedChanges is False + + # A further edit: dirty again. + py_backend._sample.setCurrentMaterialSld(2.345) + assert project.hasUnsavedChanges is True + + +# --- The save path itself ------------------------------------------------------------------------ + + +def test_save_succeeds_with_a_newly_added_or_duplicated_model_selected(py_backend): + """A model added through the collection had no calculator interface; the project's lazily + built fitter then crashed inside `as_dict`, so every save with that model selected failed.""" + project = py_backend._project + errors = [] + project.projectSaveError.connect(lambda message: errors.append(message)) + + py_backend._sample.addNewModel() + project.save() + assert errors == [] + assert project.hasUnsavedChanges is False + + py_backend._sample.duplicateSelectedModel() + project.save() + assert errors == [] + assert project.hasUnsavedChanges is False diff --git a/tests/test_py_project.py b/tests/test_py_project.py index 99201344..f5534b4e 100644 --- a/tests/test_py_project.py +++ b/tests/test_py_project.py @@ -1,4 +1,7 @@ import warnings +from datetime import datetime + +import pytest from EasyReflectometryApp.Backends.Py import project as project_module @@ -7,7 +10,8 @@ class StubProjectLogic: def __init__(self, _project_lib): self.created = False self.creation_date = '2026-03-22' - self.path = 'project.json' + self.path = 'C:/tmp/demo-project' + self.path_json = 'project.json' self.name = 'Demo' self.description = 'Desc' self.root_path = 'C:/work' @@ -17,6 +21,11 @@ def __init__(self, _project_lib): self.reset_calls = 0 self.added_samples = [] self.replaced_samples = [] + # Stands in for the model/experiment content the real fingerprint covers. + self.content_version = 0 + + def content_fingerprint(self) -> str: + return f'{self.name}|{self.description}|{self.root_path}|{self.content_version}' def create(self): self.created_calls += 1 @@ -24,23 +33,30 @@ def create(self): def load(self, path): self.loaded_paths.append(path) + self.created = True def save(self): self.saved_calls += 1 def reset(self): self.reset_calls += 1 + self.created = False def add_sample_from_orso(self, sample): self.added_samples.append(sample) + self.content_version += 1 def replace_models_from_orso(self, sample): self.replaced_samples.append(sample) + self.content_version += 1 -def _build_project(monkeypatch): +def _build_project(monkeypatch, created=False): monkeypatch.setattr(project_module, 'ProjectLogic', StubProjectLogic) - return project_module.Project(project_lib=object()) + project = project_module.Project(project_lib=object()) + if created: + project.create() + return project def test_setters_emit_only_on_change(monkeypatch, qcore_application): @@ -101,6 +117,20 @@ def test_sample_load_append_and_replace(monkeypatch, qcore_application): assert loaded['count'] == 2 +def test_sample_load_marks_the_project_dirty(monkeypatch, qcore_application): + """An imported sample is content to save, even though the relay signal it emits is not + classified as dirtying (it is the same one a project load uses).""" + project = _build_project(monkeypatch, created=True) + monkeypatch.setattr(project_module.IO, 'generalizePath', lambda path: path) + monkeypatch.setattr(project_module.orso, 'load_orso', lambda path: 'orso-data') + monkeypatch.setattr(project_module, 'load_orso_model', lambda _orso_data: 'sample-model') + assert project.hasUnsavedChanges is False + + project.sampleLoad('sample.orso', append=True) + + assert project.hasUnsavedChanges is True + + def test_sample_load_emits_warning_when_model_missing(monkeypatch, qcore_application): project = _build_project(monkeypatch) monkeypatch.setattr(project_module.IO, 'generalizePath', lambda path: path) @@ -143,3 +173,311 @@ def _raise_format_error(_path): 'Please re-create the project from its underlying data and save it again.' ] assert loaded['count'] == 0 + + +@pytest.mark.parametrize( + ('exception', 'expected_fragment'), + [ + (FileNotFoundError('nowhere/project.json'), 'does not exist'), + (PermissionError('denied'), 'could not be read'), + ], +) +def test_load_reports_file_system_failures(monkeypatch, qcore_application, exception, expected_fragment): + """The library raises for a missing or unreadable file; the slot must report, not propagate.""" + project = _build_project(monkeypatch) + monkeypatch.setattr(project_module.IO, 'generalizePath', lambda path: path) + + def _raise(_path): + raise exception + + monkeypatch.setattr(project._logic, 'load', _raise) + errors = [] + project.projectLoadError.connect(lambda msg: errors.append(msg)) + loaded = {'count': 0} + project.externalProjectLoaded.connect(lambda: loaded.__setitem__('count', loaded['count'] + 1)) + + project.load('nowhere/project.json') + + assert len(errors) == 1 + assert expected_fragment in errors[0] + assert 'nowhere/project.json' in errors[0] + assert loaded['count'] == 0 + + +def _spy_save_signals(project): + saved = [] + errors = [] + stamps = [] + project.projectSaved.connect(lambda path: saved.append(path)) + project.projectSaveError.connect(lambda msg: errors.append(msg)) + project.lastSavedChanged.connect(lambda: stamps.append(project.lastSaved)) + return saved, errors, stamps + + +def test_save_emits_projectSaved_and_stamps_last_saved(monkeypatch, qcore_application): + project = _build_project(monkeypatch) + project._logic.created = True + saved, errors, stamps = _spy_save_signals(project) + + assert project.lastSaved == '' + + project.save() + + assert saved == ['project.json'] + assert errors == [] + assert len(stamps) == 1 + assert project.lastSaved != '' + # An aware stamp is unambiguous wherever it ends up; QML parses the offset correctly. + assert datetime.fromisoformat(project.lastSaved).tzinfo is not None + + +def test_save_refuses_when_no_project_has_been_created(monkeypatch, qcore_application): + """Saving before a create would write the defaults over whatever sits at the current path.""" + project = _build_project(monkeypatch) + saved, errors, stamps = _spy_save_signals(project) + + project.save() + + assert project._logic.saved_calls == 0 + assert saved == [] + assert stamps == [] + assert len(errors) == 1 + assert 'No project has been created' in errors[0] + + +def test_save_emits_error_and_leaves_last_saved_untouched(monkeypatch, qcore_application): + project = _build_project(monkeypatch) + project._logic.created = True + + def _raise_permission_error(): + raise PermissionError('project.json is open in another program') + + monkeypatch.setattr(project._logic, 'save', _raise_permission_error) + saved, errors, stamps = _spy_save_signals(project) + + project.save() + + assert saved == [] + assert stamps == [] + assert project.lastSaved == '' + assert len(errors) == 1 + assert 'No permission to write "project.json"' in errors[0] + assert 'open in another program' in errors[0] + + +@pytest.mark.parametrize( + 'exception', + [ + ValueError('constraint depends on an unreachable parameter'), + TypeError('Object of type float32 is not JSON serializable'), + ], +) +def test_save_reports_serialization_failure(monkeypatch, qcore_application, exception): + project = _build_project(monkeypatch) + project._logic.created = True + + def _raise(): + raise exception + + monkeypatch.setattr(project._logic, 'save', _raise) + _saved, errors, _stamps = _spy_save_signals(project) + + project.save() + + assert len(errors) == 1 + assert 'cannot be serialized' in errors[0] + assert str(exception) in errors[0] + + +def test_create_reports_save_through_the_same_signals(monkeypatch, qcore_application): + project = _build_project(monkeypatch) + saved, errors, _stamps = _spy_save_signals(project) + + project.create() + + assert saved == ['project.json'] + assert errors == [] + assert project.lastSaved != '' + + +def test_create_emits_error_when_the_project_directory_already_exists(monkeypatch, qcore_application): + project = _build_project(monkeypatch) + + def _raise_file_exists(): + raise FileExistsError('Directory C:/tmp/demo-project already exists') + + monkeypatch.setattr(project._logic, 'create', _raise_file_exists) + saved, errors, _stamps = _spy_save_signals(project) + created_counts = {'created': 0} + project.createdChanged.connect(lambda: created_counts.__setitem__('created', created_counts['created'] + 1)) + + project.create() + + assert saved == [] + assert project.lastSaved == '' + assert len(errors) == 1 + # Names the directory, which is what collided, and tells the user what to change. + assert 'A project already exists at "C:/tmp/demo-project"' in errors[0] + assert 'Choose a different name or location' in errors[0] + # The UI is still told to re-read `created`, so it reflects the real state after a failure. + assert created_counts['created'] == 1 + # Nothing was created, so there is still nothing on disk for edits to differ from. + assert project.hasUnsavedChanges is False + + +def test_create_that_fails_after_the_directories_are_made_stays_saveable(monkeypatch, qcore_application): + """`ProjectLogic.create()` makes the directories and then writes the file. When only the + write fails, the library has already flipped `created`, so the UI shows a project whose file + was never written. That state must be dirty: the Save button, Ctrl+S and the close prompt are + all gated on the flag, so without it the write cannot be retried without making an unrelated + edit first, and closing the window discards the work without asking.""" + project = _build_project(monkeypatch) + saved, errors, _stamps = _spy_save_signals(project) + + def _create_then_fail_to_write(): + project._logic.created = True + raise PermissionError('project.json is read-only') + + monkeypatch.setattr(project._logic, 'create', _create_then_fail_to_write) + + project.create() + + assert project.created is True + assert saved == [] + assert len(errors) == 1 + assert project.hasUnsavedChanges is True + + # And the retry is what clears it, not an unrelated edit. + project.save() + + assert saved == ['project.json'] + assert project.hasUnsavedChanges is False + + +def test_reset_and_load_clear_the_last_saved_stamp(monkeypatch, qcore_application): + project = _build_project(monkeypatch, created=True) + monkeypatch.setattr(project_module.IO, 'generalizePath', lambda path: path) + + project.save() + assert project.lastSaved != '' + project.reset() + assert project.lastSaved == '' + + project.load('other.json') + project.save() + assert project.lastSaved != '' + project.load('other.json') + assert project.lastSaved == '' + + +def test_lifecycle_leaves_the_project_clean(monkeypatch, qcore_application): + project = _build_project(monkeypatch) + monkeypatch.setattr(project_module.IO, 'generalizePath', lambda path: path) + # Stand in for the relays that run while a project is created, loaded or reset: they emit the + # very signals dirty tracking listens to, and must not leave the project looking edited. + project.externalCreatedChanged.connect(project.markDirty) + project.externalProjectLoaded.connect(project.markDirty) + project.externalProjectReset.connect(project.markDirty) + + project.create() + assert project.hasUnsavedChanges is False + + project.setName('Edited') + assert project.hasUnsavedChanges is True + project.save() + assert project.hasUnsavedChanges is False + + project.setName('Edited again') + project.load('other.json') + assert project.hasUnsavedChanges is False + + project.setName('Edited once more') + project.reset() + assert project.hasUnsavedChanges is False + + +def test_deferred_relay_after_a_load_does_not_dirty(monkeypatch, qcore_application): + """The sample relays part of a load through a 0 ms timer, so a dirtying signal can land after + the load has finished and cleared the flag. It carries no edit, so it must not count.""" + project = _build_project(monkeypatch) + monkeypatch.setattr(project_module.IO, 'generalizePath', lambda path: path) + + project.load('other.json') + assert project.hasUnsavedChanges is False + + project.markDirty() # what the timer's constraintsChanged does once the event loop turns + + assert project.hasUnsavedChanges is False + + +def test_signal_that_changes_nothing_does_not_dirty_but_a_real_edit_does(monkeypatch, qcore_application): + """Selection changes emit the same signals as edits; only content decides.""" + project = _build_project(monkeypatch, created=True) + + project.markDirty() + assert project.hasUnsavedChanges is False + + project._logic.content_version += 1 + project.markDirty() + assert project.hasUnsavedChanges is True + + +def test_project_that_cannot_be_fingerprinted_is_treated_as_changed(monkeypatch, qcore_application): + project = _build_project(monkeypatch, created=True) + + def _raise(): + raise ValueError('constraint depends on an unreachable parameter') + + monkeypatch.setattr(project._logic, 'content_fingerprint', _raise) + + project.markDirty() + + assert project.hasUnsavedChanges is True + + +def test_failed_save_keeps_the_project_dirty(monkeypatch, qcore_application): + """The edits are still only in memory, so the close prompt must keep firing.""" + project = _build_project(monkeypatch, created=True) + + def _raise_permission_error(): + raise PermissionError('locked') + + monkeypatch.setattr(project._logic, 'save', _raise_permission_error) + project.setName('Edited') + assert project.hasUnsavedChanges is True + + project.save() + + assert project.hasUnsavedChanges is True + + +def test_edits_before_a_create_are_not_unsaved_changes(monkeypatch, qcore_application): + """Before a create there is nothing on disk for the edits to differ from, so neither the + Save button nor the close prompt has anything to offer. In particular a failed create must + not leave a "dirty" project whose "Save and exit" would overwrite the colliding one.""" + project = _build_project(monkeypatch) + project.setName('Edited') + assert project.hasUnsavedChanges is False + + def _raise_file_exists(): + raise FileExistsError('collision') + + monkeypatch.setattr(project._logic, 'create', _raise_file_exists) + project.create() + + assert project.created is False + assert project.hasUnsavedChanges is False + + +def test_unsaved_changes_notifies_only_on_transitions(monkeypatch, qcore_application): + project = _build_project(monkeypatch, created=True) + changes = [] + project.hasUnsavedChangesChanged.connect(lambda: changes.append(project.hasUnsavedChanges)) + + project.setName('One') + project.setDescription('Two') + project.setLocation('D:/three') + assert changes == [True] + + project.save() + assert changes == [True, False] diff --git a/tests/test_qml_close_behaviour.py b/tests/test_qml_close_behaviour.py new file mode 100644 index 00000000..932f8b58 --- /dev/null +++ b/tests/test_qml_close_behaviour.py @@ -0,0 +1,111 @@ +"""Runtime check of the close path ApplicationWindow.qml relies on. + +In Qt 6, `Qt.quit()` does not stop the event loop directly: it asks every top-level window to +close, which runs the window's `onClosing` handler. A handler that rejects the close while the +project is dirty therefore swallows the quit, and a dialog button that calls `Qt.quit()` after +such a handler can never exit. ApplicationWindow.qml handles this with a `discardChangesOnClose` +flag that `onClosing` consults. This test drives a window with the same handler shape through +a real `QGuiApplication` and asserts both halves: the quit is swallowed while dirty, and the +window closes once the flag is set. + +The real ApplicationWindow needs the whole application (EasyApplication components, the Globals +singletons, a backend), so the handler shape is reproduced here. The source-level test in +`test_qml_project_save_ui.py` pins that the real handler has that shape. + +A `QGuiApplication` cannot coexist with the `QCoreApplication` the other tests share, so the +harness runs in a subprocess. +""" + +import os +import subprocess +import sys +import textwrap + +import pytest + +pytest.importorskip('PySide6.QtQml') + +HARNESS = textwrap.dedent( + ''' + import os + import sys + + from PySide6.QtCore import QTimer + from PySide6.QtGui import QGuiApplication + from PySide6.QtQml import QQmlApplicationEngine + + QML = b""" + import QtQuick + + Window { + id: applicationWindow + visible: true + width: 100 + height: 100 + + property bool projectHasUnsavedChanges: true + property bool discardChangesOnClose: false + property int closingCount: 0 + + // Same shape as ApplicationWindow.qml's handler (the prompt itself is not needed here). + onClosing: function(close) { + closingCount += 1 + if (projectHasUnsavedChanges && !discardChangesOnClose) { + close.accepted = false + } + } + } + """ + + app = QGuiApplication(sys.argv) + engine = QQmlApplicationEngine() + engine.loadData(QML) + if not engine.rootObjects(): + print('HARNESS: QML failed to load') + sys.exit(2) + window = engine.rootObjects()[0] + + def quit_while_dirty(): + # What "Exit without saving" did before the fix, and what the test-mode timer does. + app.quit() + QTimer.singleShot(300, exit_with_discard_flag) + + def exit_with_discard_flag(): + # Still running: the rejected close swallowed the quit. + print(f'ALIVE after quit while dirty, closingCount={window.property("closingCount")}') + window.setProperty('discardChangesOnClose', True) + window.close() # what "Exit without saving" does now + + def report_exit(): + print(f'EXITING closingCount={window.property("closingCount")}') + + app.aboutToQuit.connect(report_exit) + QTimer.singleShot(0, quit_while_dirty) + QTimer.singleShot(10000, lambda: (print('HARNESS: timed out'), os._exit(3))) + sys.exit(app.exec()) + ''' +) + + +def test_quit_is_swallowed_while_dirty_and_the_discard_flag_lets_the_window_close(tmp_path): + script = tmp_path / 'close_harness.py' + script.write_text(HARNESS, encoding='utf-8') + environment = dict(os.environ) + environment.setdefault('QT_QPA_PLATFORM', 'offscreen') + environment.setdefault('QT_QUICK_BACKEND', 'software') + environment.setdefault('QT_LOGGING_RULES', '*.debug=false') + + completed = subprocess.run( # noqa: S603 - our own interpreter running our own script + [sys.executable, str(script)], + capture_output=True, + text=True, + timeout=60, + env=environment, + ) + + output = completed.stdout + completed.stderr + assert completed.returncode == 0, output + # First quit: onClosing ran once, rejected the close, and the application kept running. + assert 'ALIVE after quit while dirty, closingCount=1' in output, output + # With the flag set, the close is accepted and the application exits. + assert 'EXITING closingCount=2' in output, output diff --git a/tests/test_qml_project_save_ui.py b/tests/test_qml_project_save_ui.py new file mode 100644 index 00000000..ef3baf1f --- /dev/null +++ b/tests/test_qml_project_save_ui.py @@ -0,0 +1,120 @@ +"""Source-level contract checks on the project-save UI. + +These pin the API between QML and the backends (property and signal names the wrapper and the +mock must expose) and the few decisions in ApplicationWindow.qml that are easy to lose in a +refactor and invisible to the Python tests: the close prompt's gating, the discard flag the +Qt 6 quit path depends on, and the dialogs being modal. They deliberately do not restate the +implementation line by line. The runtime behaviour of the close path is covered by +`test_qml_close_behaviour.py`. +""" + +import re +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +GUI = ROOT / 'EasyReflectometryApp' / 'Gui' + + +def _application_window() -> str: + return (GUI / 'ApplicationWindow.qml').read_text(encoding='utf-8') + + +def _block(text: str, start_marker: str, lines: int = 8) -> str: + """The `lines` lines following the first occurrence of `start_marker`.""" + index = text.index(start_marker) + return '\n'.join(text[index:].splitlines()[:lines]) + + +def test_backend_wrapper_forwards_the_save_api(): + wrapper_qml = (GUI / 'Globals' / 'BackendWrapper.qml').read_text(encoding='utf-8') + + assert 'signal projectSaved(string path)' in wrapper_qml + assert 'signal projectSaveError(string message)' in wrapper_qml + assert 'activeBackend.project.projectSaved.connect(projectSaved)' in wrapper_qml + assert 'activeBackend.project.projectSaveError.connect(projectSaveError)' in wrapper_qml + assert 'readonly property string projectLastSaved' in wrapper_qml + assert 'readonly property bool projectHasUnsavedChanges' in wrapper_qml + assert 'readonly property bool projectCreated' in wrapper_qml + + +def test_mock_backend_matches_the_save_api(): + mock_qml = (ROOT / 'EasyReflectometryApp' / 'Backends' / 'Mock' / 'Project.qml').read_text(encoding='utf-8') + + assert 'signal projectSaved(string path)' in mock_qml + assert 'signal projectSaveError(string message)' in mock_qml + assert 'property string lastSaved' in mock_qml + assert 'property bool hasUnsavedChanges' in mock_qml + assert 'property bool created' in mock_qml + + +def test_status_bar_renders_the_save_stamp_in_the_user_locale(): + status_bar_qml = (GUI / 'StatusBar.qml').read_text(encoding='utf-8') + + assert "visible: Globals.BackendWrapper.projectLastSaved !== ''" in status_bar_qml + assert 'Qt.locale()' in status_bar_qml + assert 'Qt.formatTime(' in status_bar_qml + + +def test_save_is_gated_on_a_created_dirty_project_outside_a_fit(): + application_window_qml = _application_window() + + gate = _block(application_window_qml, 'readonly property bool canSaveProject', lines=3) + assert 'Globals.BackendWrapper.projectCreated' in gate + assert 'Globals.BackendWrapper.projectHasUnsavedChanges' in gate + assert '!Globals.BackendWrapper.analysisFittingRunning' in gate + # The button, the shortcut and "Save and exit" all defer to the same condition. + assert application_window_qml.count('applicationWindow.canSaveProject') >= 4 + assert 'sequences: [StandardKey.Save]' in application_window_qml + + +def test_closing_with_unsaved_changes_asks_first_and_honours_the_discard_decision(): + application_window_qml = _application_window() + + # The migration's unconditional quit must be gone (a mention in a comment does not count). + assert re.search(r'^\s*onClosing:\s*Qt\.quit\(\)\s*$', application_window_qml, re.MULTILINE) is None + handler = _block(application_window_qml, 'onClosing: function(close)', lines=8) + # Gated on a created project: before a create nothing on disk can be "unsaved", and saving + # from the prompt would overwrite whatever project sits at the chosen path. + assert 'Globals.BackendWrapper.projectCreated' in handler + assert 'Globals.BackendWrapper.projectHasUnsavedChanges' in handler + # Qt 6 runs onClosing again from Qt.quit(); without this flag "Exit without saving" loops. + assert '!applicationWindow.discardChangesOnClose' in handler + assert 'close.accepted = false' in handler + assert 'property bool discardChangesOnClose: false' in application_window_qml + + exit_button = _block(application_window_qml, "text: qsTr('Exit without saving')", lines=7) + assert 'applicationWindow.discardChangesOnClose = true' in exit_button + assert 'Qt.quit()' not in exit_button + + assert "text: qsTr('Cancel')" in application_window_qml + assert "text: qsTr('Save and exit')" in application_window_qml + + +def test_save_and_exit_only_exits_once_the_save_succeeded(): + application_window_qml = _application_window() + + assert 'property bool quitAfterSave: false' in application_window_qml + assert 'function onProjectSaved(path)' in application_window_qml + assert 'function onProjectSaveError(message)' in application_window_qml + error_handler = _block(application_window_qml, 'function onProjectSaveError(message)', lines=6) + assert 'applicationWindow.quitAfterSave = false' in error_handler + + +def test_test_mode_quit_cannot_be_swallowed_by_the_close_prompt(): + application_window_qml = _application_window() + + test_mode_end = _block(application_window_qml, "'*** TEST MODE 30 s DELAYED END ***'", lines=5) + assert 'applicationWindow.discardChangesOnClose = true' in test_mode_end + assert 'Qt.quit()' in test_mode_end + + +def test_the_prompt_and_error_dialogs_are_modal(): + """EaElements.Dialog is modeless by default; these must block the window behind them.""" + application_window_qml = _application_window() + + for dialog_id in ('closeDialog', 'projectSaveErrorDialog', 'resetStateDialog'): + assert 'modal: true' in _block(application_window_qml, f'id: {dialog_id}', lines=8), dialog_id + + +def test_reset_dialog_names_the_unsaved_work_at_risk(): + assert 'The project has unsaved changes that will be lost.' in _application_window()