Skip to content

Commit 6219fba

Browse files
georgweissgeorgweiss
committed
Refacotring to improve maintainability
1 parent bc0a73d commit 6219fba

File tree

15 files changed

+82
-144
lines changed

15 files changed

+82
-144
lines changed

‎app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/SaveAndRestoreInstance.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020

2121
import javafx.fxml.FXMLLoader;
2222
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreController;
23-
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreService;
2423
import org.phoebus.framework.nls.NLS;
2524
import org.phoebus.framework.persistence.Memento;
2625
import org.phoebus.framework.spi.AppDescriptor;
@@ -83,11 +82,11 @@ public void openResource(URI uri) {
8382
saveAndRestoreController.openResource(uri);
8483
}
8584

86-
public void secureStoreChanged(List<ScopedAuthenticationToken> validTokens){
85+
public void secureStoreChanged(List<ScopedAuthenticationToken> validTokens) {
8786
saveAndRestoreController.secureStoreChanged(validTokens);
8887
}
8988

90-
public void raise(){
89+
public void raise() {
9190
dockItem.select();
9291
}
9392
}

‎app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/client/SaveAndRestoreClientImpl.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,9 @@
3636
import java.net.CookieManager;
3737
import java.net.CookiePolicy;
3838
import java.net.URI;
39-
import java.net.URLEncoder;
4039
import java.net.http.HttpClient;
4140
import java.net.http.HttpRequest;
4241
import java.net.http.HttpResponse;
43-
import java.nio.charset.StandardCharsets;
4442
import java.time.Duration;
4543
import java.util.Base64;
4644
import java.util.List;
@@ -588,10 +586,10 @@ public UserData authenticate(String userName, String password) {
588586
String stringBuilder = Preferences.jmasarServiceUrl +
589587
"/login";
590588
HttpRequest request = HttpRequest.newBuilder()
591-
.uri(URI.create(stringBuilder))
592-
.header("Content-Type", CONTENT_TYPE_JSON)
593-
.POST(HttpRequest.BodyPublishers.ofString(OBJECT_MAPPER.writeValueAsString(new LoginCredentials(userName, password))))
594-
.build();
589+
.uri(URI.create(stringBuilder))
590+
.header("Content-Type", CONTENT_TYPE_JSON)
591+
.POST(HttpRequest.BodyPublishers.ofString(OBJECT_MAPPER.writeValueAsString(new LoginCredentials(userName, password))))
592+
.build();
595593
HttpResponse<String> response = CLIENT.send(request, HttpResponse.BodyHandlers.ofString());
596594
return OBJECT_MAPPER.readValue(response.body(), UserData.class);
597595
} catch (Exception e) {

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,12 @@
3333
public abstract class SaveAndRestoreBaseController {
3434

3535
protected final SimpleStringProperty userIdentity = new SimpleStringProperty();
36+
protected final WebSocketClientService webSocketClientService;
37+
protected final SaveAndRestoreService saveAndRestoreService;
3638

3739
public SaveAndRestoreBaseController() {
40+
this.webSocketClientService = WebSocketClientService.getInstance();
41+
this.saveAndRestoreService = SaveAndRestoreService.getInstance();
3842
try {
3943
SecureStore secureStore = new SecureStore();
4044
ScopedAuthenticationToken token =
@@ -69,6 +73,11 @@ public SimpleStringProperty getUserIdentity() {
6973
* Default no-op implementation of a handler for {@link SaveAndRestoreWebSocketMessage}s.
7074
* @param webSocketMessage See {@link SaveAndRestoreWebSocketMessage}
7175
*/
72-
public void handleWebSocketMessage(SaveAndRestoreWebSocketMessage<?> webSocketMessage){
76+
protected void handleWebSocketMessage(SaveAndRestoreWebSocketMessage<?> webSocketMessage){
77+
}
78+
79+
80+
protected boolean handleTabClosed(){
81+
return true;
7382
}
7483
}

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

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,6 @@ public class SaveAndRestoreController extends SaveAndRestoreBaseController
168168
@FXML
169169
private VBox treeViewPane;
170170

171-
private SaveAndRestoreService saveAndRestoreService;
172-
private WebSocketClientService webSocketClientService;
173-
174171
private final ObjectMapper objectMapper = new ObjectMapper();
175172
protected MultipleSelectionModel<TreeItem<Node>> browserSelectionModel;
176173
private static final String TREE_STATE = "tree_state";
@@ -255,7 +252,6 @@ public void initialize(URL url, ResourceBundle resourceBundle) {
255252
// Tree items are first compared on type, then on name (case-insensitive).
256253
treeNodeComparator = Comparator.comparing(TreeItem::getValue);
257254

258-
saveAndRestoreService = SaveAndRestoreService.getInstance();
259255
treeView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
260256
treeView.getStylesheets().add(getClass().getResource("/save-and-restore-style.css").toExternalForm());
261257

@@ -359,7 +355,6 @@ public Filter fromString(String s) {
359355

360356
webSocketTrackerLabel.textProperty().bind(webSocketTrackerText);
361357

362-
webSocketClientService = WebSocketClientService.getInstance();
363358
webSocketClientService.addWebSocketMessageHandler(this);
364359
webSocketClientService.setConnectCallback(() -> Platform.runLater(() -> webSocketTrackerText.setValue("Web Socket Connected")));
365360
webSocketClientService.setDisconnectCallback(() -> Platform.runLater(() -> webSocketTrackerText.setValue("Web Socket Disconnected")));
@@ -927,11 +922,11 @@ public void saveLocalState() {
927922
}
928923
}
929924

930-
public void handleTabClosed() {
925+
@Override
926+
public boolean handleTabClosed() {
931927
saveLocalState();
932928
webSocketClientService.closeWebSocket();
933-
webSocketClientService.removeWebSocketMessageHandler(this);
934-
webSocketClientService.close();
929+
return true;
935930
}
936931

937932
/**

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import javafx.scene.control.MenuItem;
2525
import javafx.scene.control.Tab;
2626
import javafx.scene.image.ImageView;
27+
import org.phoebus.applications.saveandrestore.model.websocket.SaveAndRestoreWebSocketMessage;
2728
import org.phoebus.applications.saveandrestore.ui.snapshot.SnapshotTab;
2829
import org.phoebus.security.tokens.ScopedAuthenticationToken;
2930
import org.phoebus.ui.javafx.ImageCache;
@@ -34,9 +35,10 @@
3435
/**
3536
* Base class for save-n-restore {@link Tab}s containing common functionality.
3637
*/
37-
public abstract class SaveAndRestoreTab extends Tab {
38+
public abstract class SaveAndRestoreTab extends Tab implements WebSocketMessageHandler {
3839

3940
protected SaveAndRestoreBaseController controller;
41+
protected WebSocketClientService webSocketClientService;
4042

4143
public SaveAndRestoreTab() {
4244
ContextMenu contextMenu = new ContextMenu();
@@ -60,6 +62,18 @@ public SaveAndRestoreTab() {
6062

6163
contextMenu.getItems().addAll(closeAll, closeOthers);
6264
setContextMenu(contextMenu);
65+
66+
webSocketClientService = WebSocketClientService.getInstance();
67+
68+
setOnCloseRequest(event -> {
69+
if (!controller.handleTabClosed()) {
70+
event.consume();
71+
} else {
72+
webSocketClientService.removeWebSocketMessageHandler(this);
73+
}
74+
});
75+
76+
webSocketClientService.addWebSocketMessageHandler(this);
6377
}
6478

6579
/**
@@ -70,4 +84,9 @@ public SaveAndRestoreTab() {
7084
public void secureStoreChanged(List<ScopedAuthenticationToken> validTokens) {
7185
controller.secureStoreChanged(validTokens);
7286
}
87+
88+
@Override
89+
public void handleWebSocketMessage(SaveAndRestoreWebSocketMessage<?> saveAndRestoreWebSocketMessage) {
90+
91+
}
7392
}

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

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,10 @@
1919
import java.util.ArrayList;
2020
import java.util.Collections;
2121
import java.util.List;
22-
import java.util.logging.Logger;
2322

2423
public class WebSocketClientService {
2524

2625
private final List<WebSocketMessageHandler> webSocketMessageHandlers = Collections.synchronizedList(new ArrayList<>());
27-
private static final Logger LOG = Logger.getLogger(WebSocketClientService.class.getName());
2826

2927
private static WebSocketClientService instance;
3028
private final WebSocketClient webSocketClient;
@@ -36,7 +34,7 @@ private WebSocketClientService() {
3634
String schema = baseUrl.startsWith("https") ? "wss" : "ws";
3735
String webSocketUrl = schema + baseUrl.substring(baseUrl.indexOf("://")) + "/web-socket";
3836
URI webSocketUri = URI.create(webSocketUrl);
39-
webSocketClient = new WebSocketClient(webSocketUri, this::handleWebSocketMessage);
37+
webSocketClient = new WebSocketClient(webSocketUri, this::handleWebSocketMessage);
4038
objectMapper = new ObjectMapper();
4139
objectMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
4240
objectMapper.registerModule(new JavaTimeModule());
@@ -47,34 +45,31 @@ private WebSocketClientService() {
4745
objectMapper.registerModule(module);
4846
}
4947

50-
public static WebSocketClientService getInstance(){
51-
if(instance == null){
48+
public static WebSocketClientService getInstance() {
49+
if (instance == null) {
5250
instance = new WebSocketClientService();
5351
}
5452
return instance;
5553
}
5654

57-
public void connect(){
55+
public void connect() {
5856
webSocketClient.connect();
5957
}
6058

61-
public void closeWebSocket(){
59+
public void closeWebSocket() {
60+
webSocketMessageHandlers.clear();
6261
webSocketClient.close("Application shutdown");
6362
}
6463

65-
public void openWebSocket(){
66-
webSocketClient.connect();
67-
}
68-
69-
public void addWebSocketMessageHandler(WebSocketMessageHandler webSocketMessageHandler){
64+
public void addWebSocketMessageHandler(WebSocketMessageHandler webSocketMessageHandler) {
7065
webSocketMessageHandlers.add(webSocketMessageHandler);
7166
}
7267

73-
public void removeWebSocketMessageHandler(WebSocketMessageHandler webSocketMessageHandler){
68+
public void removeWebSocketMessageHandler(WebSocketMessageHandler webSocketMessageHandler) {
7469
webSocketMessageHandlers.remove(webSocketMessageHandler);
7570
}
7671

77-
private void handleWebSocketMessage(CharSequence charSequence){
72+
private void handleWebSocketMessage(CharSequence charSequence) {
7873
try {
7974
SaveAndRestoreWebSocketMessage saveAndRestoreWebSocketMessage =
8075
objectMapper.readValue(charSequence.toString(), SaveAndRestoreWebSocketMessage.class);
@@ -85,15 +80,11 @@ private void handleWebSocketMessage(CharSequence charSequence){
8580
}
8681
}
8782

88-
public void setConnectCallback(Runnable connectCallback){
83+
public void setConnectCallback(Runnable connectCallback) {
8984
webSocketClient.setConnectCallback(connectCallback);
9085
}
9186

92-
public void setDisconnectCallback(Runnable disconnectCallback){
87+
public void setDisconnectCallback(Runnable disconnectCallback) {
9388
webSocketClient.setDisconnectCallback(disconnectCallback);
9489
}
95-
96-
public void close(){
97-
webSocketClient.close("Application shutdown");
98-
}
9990
}

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

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,6 @@
6161
import org.phoebus.applications.saveandrestore.model.websocket.MessageType;
6262
import org.phoebus.applications.saveandrestore.model.websocket.SaveAndRestoreWebSocketMessage;
6363
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreBaseController;
64-
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreService;
65-
import org.phoebus.applications.saveandrestore.ui.WebSocketClientService;
6664
import org.phoebus.applications.saveandrestore.ui.WebSocketMessageHandler;
6765
import org.phoebus.core.types.ProcessVariable;
6866
import org.phoebus.framework.jobs.JobManager;
@@ -163,9 +161,6 @@ public class ConfigurationController extends SaveAndRestoreBaseController implem
163161
@SuppressWarnings("unused")
164162
private Pane addPVsPane;
165163

166-
private SaveAndRestoreService saveAndRestoreService;
167-
private WebSocketClientService webSocketClientService;
168-
169164
private final ObservableList<ConfigPvEntry> configurationEntries = FXCollections.observableArrayList();
170165

171166
private final SimpleBooleanProperty selectionEmpty = new SimpleBooleanProperty(false);
@@ -197,9 +192,6 @@ public ConfigurationController(ConfigurationTab configurationTab) {
197192
@FXML
198193
public void initialize() {
199194

200-
saveAndRestoreService = SaveAndRestoreService.getInstance();
201-
webSocketClientService = WebSocketClientService.getInstance();
202-
203195
pvTable.editableProperty().bind(userIdentity.isNull().not());
204196
pvTable.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
205197
pvTable.getSelectionModel().selectedItemProperty().addListener((obs, ov, nv) -> selectionEmpty.set(nv == null));
@@ -526,7 +518,8 @@ public synchronized void loadConfiguration(final Node node) {
526518
* @return <code>true</code> if content is not dirty or user chooses to close anyway,
527519
* otherwise <code>false</code>.
528520
*/
529-
public boolean handleConfigurationTabClosed() {
521+
@Override
522+
public boolean handleTabClosed() {
530523
if (dirty.get()) {
531524
Alert alert = new Alert(Alert.AlertType.CONFIRMATION);
532525
alert.setTitle(Messages.closeTabPrompt);

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

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,7 @@
2626
import org.phoebus.applications.saveandrestore.model.websocket.MessageType;
2727
import org.phoebus.applications.saveandrestore.model.websocket.SaveAndRestoreWebSocketMessage;
2828
import org.phoebus.applications.saveandrestore.ui.ImageRepository;
29-
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreService;
3029
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreTab;
31-
import org.phoebus.applications.saveandrestore.ui.WebSocketClientService;
3230
import org.phoebus.applications.saveandrestore.ui.WebSocketMessageHandler;
3331
import org.phoebus.framework.nls.NLS;
3432

@@ -39,10 +37,6 @@
3937
public class ConfigurationTab extends SaveAndRestoreTab implements WebSocketMessageHandler {
4038

4139
public ConfigurationTab() {
42-
configure();
43-
}
44-
45-
private void configure() {
4640
try {
4741
FXMLLoader loader = new FXMLLoader();
4842
ResourceBundle resourceBundle = NLS.getMessages(Messages.class);
@@ -66,19 +60,7 @@ private void configure() {
6660
} catch (Exception e) {
6761
Logger.getLogger(ConfigurationTab.class.getName())
6862
.log(Level.SEVERE, "Failed to load fxml", e);
69-
return;
7063
}
71-
72-
setOnCloseRequest(event -> {
73-
if (!((ConfigurationController) controller).handleConfigurationTabClosed()) {
74-
event.consume();
75-
}
76-
else{
77-
WebSocketClientService.getInstance().removeWebSocketMessageHandler(this);
78-
}
79-
});
80-
81-
WebSocketClientService.getInstance().addWebSocketMessageHandler(this);
8264
}
8365

8466
/**

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@
5555
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreBaseController;
5656
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreController;
5757
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreService;
58-
import org.phoebus.applications.saveandrestore.ui.WebSocketClientService;
5958
import org.phoebus.applications.saveandrestore.ui.WebSocketMessageHandler;
6059
import org.phoebus.framework.jobs.JobManager;
6160
import org.phoebus.security.tokens.ScopedAuthenticationToken;
@@ -182,9 +181,6 @@ public class SearchAndFilterViewController extends SaveAndRestoreBaseController
182181

183182
private final SimpleStringProperty filterNameProperty = new SimpleStringProperty();
184183

185-
private final SaveAndRestoreService saveAndRestoreService;
186-
private final WebSocketClientService webSocketClientService;
187-
188184
private final SimpleStringProperty query = new SimpleStringProperty();
189185

190186
private final SimpleStringProperty pvNamesProperty = new SimpleStringProperty();
@@ -222,8 +218,6 @@ public class SearchAndFilterViewController extends SaveAndRestoreBaseController
222218

223219
public SearchAndFilterViewController(SaveAndRestoreController saveAndRestoreController) {
224220
this.saveAndRestoreController = saveAndRestoreController;
225-
this.saveAndRestoreService = SaveAndRestoreService.getInstance();
226-
this.webSocketClientService = WebSocketClientService.getInstance();
227221
}
228222

229223
@FXML

0 commit comments

Comments
 (0)