Skip to content

Commit 85eef3c

Browse files
georgweissgeorgweiss
committed
Refactoring of controller to facilitate maintenance
1 parent eec7e6a commit 85eef3c

File tree

1 file changed

+53
-50
lines changed

1 file changed

+53
-50
lines changed

‎app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/SnapshotController.java‎

Lines changed: 53 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,23 @@ public class SnapshotController extends SaveAndRestoreBaseController implements
319319
private final SimpleBooleanProperty showDeltaPercentage = new SimpleBooleanProperty(false);
320320
private final SimpleBooleanProperty compareViewEnabled = new SimpleBooleanProperty(false);
321321
private final SimpleObjectProperty<ActionResult> actionResultProperty = new SimpleObjectProperty<>(ActionResult.PENDING);
322+
/**
323+
* Used to control the disable state on the Take Snapshot button.
324+
*/
325+
private final SimpleObjectProperty<NodeType> nodeTypeProperty = new SimpleObjectProperty<>(NodeType.SNAPSHOT);
322326

323327
private SnapshotUtil snapshotUtil;
328+
/**
329+
* Used to disable portions of the UI when long-lasting operations are in progress, e.g.
330+
* take snapshot or save snapshot.
331+
*/
332+
protected final SimpleBooleanProperty disabledUi = new SimpleBooleanProperty(false);
333+
334+
/**
335+
* The final {@link Snapshot} object holding the data for this controller. When user performs operations (loading,
336+
* taking and saving snapshots), the fields of this object are updated accordingly.
337+
*/
338+
private final Snapshot snapshot = new Snapshot();
324339

325340

326341
/**
@@ -339,28 +354,19 @@ public SnapshotController(SnapshotTab snapshotTab) {
339354
snapshotTab.setGraphic(imageView);
340355
}
341356

342-
/**
343-
* Used to disable portions of the UI when long-lasting operations are in progress, e.g.
344-
* take snapshot or save snapshot.
345-
*/
346-
protected final SimpleBooleanProperty disabledUi = new SimpleBooleanProperty(false);
347357

348-
private final SimpleObjectProperty<Snapshot> snapshotProperty = new SimpleObjectProperty<>();
349358

350359
@FXML
351360
public void initialize() {
352361

362+
Node snapshotNode = Node.builder().nodeType(NodeType.SNAPSHOT).build();
363+
snapshot.setSnapshotNode(snapshotNode);
364+
353365
// Locate registered SaveAndRestoreEventReceivers
354366
eventReceivers = ServiceLoader.load(SaveAndRestoreEventReceiver.class);
355367
progressIndicator.visibleProperty().bind(disabledUi);
356368
disabledUi.addListener((observable, oldValue, newValue) -> borderPane.setDisable(newValue));
357369

358-
snapshotProperty.addListener((ob, o, n) -> {
359-
if (n != null) {
360-
showSnapshotInTable(n);
361-
}
362-
});
363-
364370
snapshotDataDirty.addListener((obs, o, n) -> {
365371
if (n && !tabTitleProperty.get().startsWith("* ")) {
366372
Platform.runLater(() -> tabTitleProperty.setValue("* " + tabTitleProperty.get()));
@@ -378,22 +384,21 @@ public void initialize() {
378384
snapshotLastModifiedLabel.textProperty().bind(lastModifiedDateTextProperty);
379385

380386
takeSnapshotButton.disableProperty().bind(Bindings.createBooleanBinding(() ->
381-
(snapshotProperty.isNotNull().get() && !snapshotProperty.get().getSnapshotNode().getNodeType().equals(NodeType.SNAPSHOT)) ||
382-
userIdentity.isNull().get(), snapshotProperty, userIdentity));
387+
!nodeTypeProperty.get().equals(NodeType.SNAPSHOT) ||
388+
userIdentity.isNull().get(), nodeTypeProperty, userIdentity));
383389

384390
snapshotNameProperty.addListener(((observableValue, oldValue, newValue) ->
385-
snapshotDataDirty.set(newValue != null && (snapshotProperty.isNotNull().get() && !newValue.equals(snapshotProperty.get().getSnapshotNode().getName())))));
391+
snapshotDataDirty.set(newValue != null && !newValue.equals(snapshot.getSnapshotNode().getName()))));
386392
snapshotCommentProperty.addListener(((observableValue, oldValue, newValue) ->
387-
snapshotDataDirty.set(newValue != null && (snapshotProperty.isNotNull().get() && !newValue.equals(snapshotProperty.get().getSnapshotNode().getDescription())))));
393+
snapshotDataDirty.set(newValue != null && !newValue.equals(snapshot.getSnapshotNode().getDescription()))));
388394

389395
saveSnapshotButton.disableProperty().bind(Bindings.createBooleanBinding(() ->
390396
// TODO: support save (=update) a composite snapshot from the snapshot view. In the meanwhile, disable save button.
391-
snapshotProperty.isNull().get() ||
392397
snapshotDataDirty.not().get() ||
393398
snapshotNameProperty.isEmpty().get() ||
394399
snapshotCommentProperty.isEmpty().get() ||
395400
userIdentity.isNull().get(),
396-
snapshotProperty, snapshotDataDirty, snapshotNameProperty, snapshotCommentProperty, userIdentity));
401+
snapshotDataDirty, snapshotNameProperty, snapshotCommentProperty, userIdentity));
397402

398403
// Do not show the create log checkbox if no event receivers have been registered
399404
logAction.visibleProperty().set(ServiceLoader.load(SaveAndRestoreEventReceiver.class).iterator().hasNext());
@@ -480,13 +485,6 @@ public void initialize() {
480485
.addListener((a, o, n) ->
481486
hideEqualItems());
482487

483-
snapshotProperty.addListener((ob, old, snapshot) -> {
484-
if (snapshot != null){
485-
showSnapshotInTable(snapshot);
486-
updateUi(snapshot.getSnapshotNode());
487-
}
488-
});
489-
490488
logAction.selectedProperty().bindBidirectional(logActionProperty);
491489

492490
readPVs.setUserData(SnapshotMode.READ_PVS);
@@ -709,15 +707,17 @@ public void updateItem(ActionResult actionResult, boolean empty) {
709707
webSocketClientService.addWebSocketMessageHandler(this);
710708
}
711709

712-
private void updateUi(Node node) {
710+
private void updateUi() {
713711
Platform.runLater(() -> {
712+
Node node = snapshot.getSnapshotNode();
714713
snapshotNameProperty.set(node.getName());
715714
snapshotCommentProperty.set(node.getDescription());
716715
createdDateTextProperty.set(node.getCreated() != null ? TimestampFormats.SECONDS_FORMAT.format(node.getCreated().toInstant()) : null);
717716
lastModifiedDateTextProperty.set(node.getLastModified() != null ? TimestampFormats.SECONDS_FORMAT.format(node.getLastModified().toInstant()) : null);
718717
createdByTextProperty.set(node.getUserName());
719718
filterToolbar.disableProperty().set(node.getName() == null);
720719
});
720+
showSnapshotInTable();
721721
}
722722

723723
private void parseAndUpdateThreshold(String value) {
@@ -727,7 +727,7 @@ private void parseAndUpdateThreshold(String value) {
727727
double parsedNumber;
728728
try {
729729
parsedNumber = Double.parseDouble(value.trim());
730-
updateThreshold(snapshotProperty.get(), parsedNumber);
730+
updateThreshold(parsedNumber);
731731
} catch (Exception e) {
732732
thresholdSpinner.getEditor().getStyleClass().add("input-error");
733733
thresholdSpinner.setTooltip(new Tooltip(Messages.toolTipMultiplierSpinner));
@@ -755,14 +755,10 @@ public void initializeViewForNewSnapshot(Node configurationNode) {
755755
}
756756
showLiveReadbackButton.setSelected(configurationHasReadbackPvs(configurationData));
757757
List<ConfigPv> configPvs = configurationData.getPvList();
758-
Snapshot snapshot = new Snapshot();
759-
snapshot.setSnapshotNode(Node.builder().nodeType(NodeType.SNAPSHOT).build());
760758
SnapshotData snapshotData = new SnapshotData();
761759
snapshotData.setSnapshotItems(configurationToSnapshotItems(configPvs));
762-
snapshot.setSnapshotData(snapshotData);
763-
snapshotProperty.set(snapshot);
764-
updateUi(snapshot.getSnapshotNode());
765-
showSnapshotInTable(snapshot);
760+
this.snapshot.setSnapshotData(snapshotData);
761+
updateUi();
766762
setTabImage(snapshot.getSnapshotNode());
767763
});
768764
}
@@ -781,7 +777,9 @@ public void takeSnapshot() {
781777
disabledUi.set(false);
782778
if (snapshot.isPresent()) {
783779
snapshotDataDirty.set(true);
784-
snapshotProperty.set(snapshot.get());
780+
this.snapshot.setSnapshotNode(snapshot.get().getSnapshotNode());
781+
this.snapshot.setSnapshotData(snapshot.get().getSnapshotData());
782+
updateUi();
785783
}
786784
});
787785
}
@@ -790,16 +788,16 @@ public void takeSnapshot() {
790788
public void saveSnapshot(ActionEvent actionEvent) {
791789
disabledUi.set(true);
792790
JobManager.schedule("Save Snapshot", monitor -> {
793-
List<SnapshotItem> snapshotItems = snapshotProperty.get().getSnapshotData().getSnapshotItems();
791+
List<SnapshotItem> snapshotItems = snapshot.getSnapshotData().getSnapshotItems();
794792
SnapshotData snapshotData = new SnapshotData();
795793
snapshotData.setSnapshotItems(snapshotItems);
796-
Snapshot snapshot = snapshotProperty.get();
794+
this.snapshot.setSnapshotData(snapshotData);
797795
Node snapshotNode =
798796
Node.builder()
799797
.nodeType(NodeType.SNAPSHOT)
800798
.name(snapshotNameProperty.get())
801799
.description(snapshotCommentProperty.get())
802-
.uniqueId(snapshotProperty.get().getSnapshotNode().getUniqueId())
800+
.uniqueId(snapshot.getSnapshotNode().getUniqueId())
803801
.build();
804802
snapshot.setSnapshotNode(snapshotNode);
805803

@@ -841,7 +839,7 @@ private List<SnapshotItem> configurationToSnapshotItems(List<ConfigPv> configPvs
841839

842840

843841
private void updateLoadedSnapshot(TableEntry rowValue, VType newValue) {
844-
snapshotProperty.get().getSnapshotData().getSnapshotItems().stream()
842+
snapshot.getSnapshotData().getSnapshotItems().stream()
845843
.filter(item -> item.getConfigPv().equals(rowValue.getConfigPv()))
846844
.findFirst()
847845
.ifPresent(item -> {
@@ -915,8 +913,10 @@ public void loadSnapshot(Node snapshotNode) {
915913
JobManager.schedule("Load snapshot items", monitor -> {
916914
try {
917915
Snapshot snapshot = getSnapshotFromService(snapshotNode);
918-
snapshotProperty.set(snapshot);
916+
this.snapshot.setSnapshotNode(snapshot.getSnapshotNode());
917+
this.snapshot.setSnapshotData(snapshot.getSnapshotData());
919918
Platform.runLater(() -> {
919+
nodeTypeProperty.set(snapshot.getSnapshotNode().getNodeType());
920920
showLiveReadbackButton.setSelected(configurationHasReadbackPvs(snapshot.getSnapshotData()));
921921
snapshotRestorableProperty.set(true);
922922
selectedColumn.visibleProperty().set(true);
@@ -926,7 +926,7 @@ public void loadSnapshot(Node snapshotNode) {
926926
snapshotRestorableProperty.set(true);
927927
setTabImage(snapshotNode);
928928
});
929-
updateUi(snapshot.getSnapshotNode());
929+
updateUi();
930930
} finally {
931931
disabledUi.set(false);
932932
}
@@ -939,7 +939,7 @@ public void restore() {
939939
restore(restoreModeProperty.get(), restoreResultList -> {
940940
disabledUi.setValue(false);
941941
if (logActionProperty.get()) {
942-
eventReceivers.forEach(r -> r.snapshotRestored(snapshotProperty.get().getSnapshotNode(), restoreResultList, this::showLoggingError));
942+
eventReceivers.forEach(r -> r.snapshotRestored(snapshot.getSnapshotNode(), restoreResultList, this::showLoggingError));
943943
}
944944
});
945945
}
@@ -1079,7 +1079,7 @@ private void takeSnapshotFromArchiver(Consumer<Optional<Snapshot>> consumer) {
10791079
}
10801080
analyzeTakeSnapshotResult(snapshotItems);
10811081
Snapshot snapshot = new Snapshot();
1082-
snapshot.setSnapshotNode(Node.builder().nodeType(NodeType.SNAPSHOT).created(new Date(time.get().toEpochMilli())).build());
1082+
snapshot.setSnapshotNode(Node.builder().name(Messages.archiver).nodeType(NodeType.SNAPSHOT).created(new Date(time.get().toEpochMilli())).build());
10831083
SnapshotData snapshotData = new SnapshotData();
10841084
snapshotData.setUniqueId("anonymous");
10851085
snapshotData.setSnapshotItems(snapshotItems);
@@ -1164,7 +1164,7 @@ private void analyzeTakeSnapshotResult(List<SnapshotItem> snapshotItems) {
11641164
});
11651165
}
11661166

1167-
private void updateThreshold(Snapshot snapshot, double threshold) {
1167+
private void updateThreshold(double threshold) {
11681168
snapshot.getSnapshotData().getSnapshotItems().forEach(item -> {
11691169
VType vtype = item.getValue();
11701170
VNumber diffVType;
@@ -1196,7 +1196,7 @@ private void updateThreshold(Snapshot snapshot, double threshold) {
11961196
* @param multiplier The (double) factor used to change the snapshot set-points used in restore operation.
11971197
*/
11981198
private void updateSnapshotValues(double multiplier) {
1199-
snapshotProperty.get().getSnapshotData().getSnapshotItems()
1199+
snapshot.getSnapshotData().getSnapshotItems()
12001200
.forEach(item -> {
12011201
TableEntry tableEntry = tableEntryItems.get(item.getConfigPv().getPvName());
12021202
VType vtype = tableEntry.storedSnapshotValue().get();
@@ -1281,14 +1281,14 @@ private void hideEqualItems() {
12811281
*/
12821282
private void restore(RestoreMode restoreMode, Consumer<List<RestoreResult>> completion) {
12831283
actionResultProperty.set(ActionResult.PENDING);
1284-
JobManager.schedule("Restore snapshot " + snapshotProperty.get().getSnapshotNode().getName(), monitor -> {
1284+
JobManager.schedule("Restore snapshot " + snapshot.getSnapshotNode().getName(), monitor -> {
12851285
List<RestoreResult> restoreResultList = null;
12861286
try {
12871287
switch (restoreMode) {
12881288
case CLIENT_RESTORE ->
1289-
restoreResultList = snapshotUtil.restore(getSnapshotItemsToRestore(snapshotProperty.get()));
1289+
restoreResultList = snapshotUtil.restore(getSnapshotItemsToRestore(snapshot));
12901290
case SERVICE_RESTORE ->
1291-
restoreResultList = SaveAndRestoreService.getInstance().restore(getSnapshotItemsToRestore(snapshotProperty.get()));
1291+
restoreResultList = SaveAndRestoreService.getInstance().restore(getSnapshotItemsToRestore(snapshot));
12921292
}
12931293
} catch (Exception e) {
12941294
Platform.runLater(() -> {
@@ -1486,7 +1486,7 @@ private void addSnapshot(Snapshot snapshot) {
14861486
updateTable(null);
14871487
}
14881488

1489-
private void showSnapshotInTable(Snapshot snapshot) {
1489+
private void showSnapshotInTable() {
14901490
if (snapshots.isEmpty()) {
14911491
snapshots.add(snapshot);
14921492
} else {
@@ -1549,7 +1549,7 @@ private void updateTable(List<TableEntry> entries) {
15491549
}
15501550

15511551
public Snapshot getSnapshot() {
1552-
return snapshotProperty.get();
1552+
return snapshot;
15531553
}
15541554

15551555
public Node getConfigurationNode() {
@@ -1564,7 +1564,10 @@ private int measureStringWidth(String text, Font font) {
15641564
return (int) mText.getLayoutBounds().getWidth();
15651565
}
15661566

1567-
1567+
/**
1568+
* Attempts to connect to all the PVs of the configuration/snapshot and binds the created {@link SaveAndRestorePV} objects
1569+
* to the {@link TableEntry} objects matched on PV name.
1570+
*/
15681571
private void connectPVs() {
15691572
JobManager.schedule("Connect PVs", monitor -> tableEntryItems.values().forEach(e -> {
15701573
SaveAndRestorePV pv = pvs.get(e.getConfigPv().getPvName());

0 commit comments

Comments
 (0)