Skip to content

Commit cdf157f

Browse files
georgweissgeorgweiss
committed
Fixing log entry grouping bug
1 parent fbb6732 commit cdf157f

File tree

6 files changed

+87
-43
lines changed

6 files changed

+87
-43
lines changed

‎app/logbook/olog/client-es/src/main/java/org/phoebus/applications/logbook/OlogESLogbook.java‎

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,57 +1,63 @@
11
package org.phoebus.applications.logbook;
22

3-
import java.util.logging.Level;
4-
import java.util.logging.Logger;
5-
63
import org.phoebus.logbook.LogClient;
74
import org.phoebus.logbook.LogFactory;
8-
import org.phoebus.olog.es.api.OlogClient;
95
import org.phoebus.olog.es.api.OlogClient.OlogClientBuilder;
106
import org.phoebus.security.tokens.SimpleAuthenticationToken;
7+
8+
import java.util.logging.Level;
9+
import java.util.logging.Logger;
10+
1111
/**
1212
* Logbook client for the new es based olog.
1313
* TODO: in the future this client would replace the old olog client
14-
* @author kunal
1514
*
15+
* @author kunal
1616
*/
1717
public class OlogESLogbook implements LogFactory {
1818

1919
private static final Logger logger = Logger.getLogger(OlogESLogbook.class.getName());
2020
private static final String ID = "olog-es";
21-
private OlogClient oLogClient;
2221

2322
@Override
2423
public String getId() {
2524
return ID;
2625
}
2726

27+
/**
28+
*
29+
* @return A fresh instance of the client. Instead of maintaining a client reference in this class,
30+
* a new instance is return since the user may have signed out or signed in thus invalidating
31+
* the authentication token.
32+
*/
2833
@Override
2934
public LogClient getLogClient() {
30-
if (oLogClient == null) {
31-
try {
32-
oLogClient = OlogClientBuilder.serviceURL().create();
33-
} catch (Exception e) {
34-
logger.log(Level.SEVERE, "Failed to create olog es client", e);
35-
}
35+
try {
36+
return OlogClientBuilder.serviceURL().create();
37+
} catch (Exception e) {
38+
logger.log(Level.SEVERE, "Failed to create olog es client", e);
3639
}
37-
return oLogClient;
40+
return null;
3841
}
3942

43+
/**
44+
* @param authToken An authentication token.
45+
* @return A fresh instance of the client. Instead of maintaining a client reference in this class,
46+
* a new instance to force usage of the specified authentication token.
47+
*/
4048
@Override
4149
public LogClient getLogClient(Object authToken) {
4250
try {
4351
if (authToken instanceof SimpleAuthenticationToken) {
4452
SimpleAuthenticationToken token = (SimpleAuthenticationToken) authToken;
4553
return OlogClientBuilder.serviceURL().withHTTPAuthentication(true).username(token.getUsername()).password(token.getPassword())
4654
.create();
47-
} else if (oLogClient == null) {
48-
oLogClient = OlogClientBuilder.serviceURL().create();
49-
55+
} else {
56+
return getLogClient();
5057
}
5158
} catch (Exception e) {
5259
logger.log(Level.SEVERE, "Failed to create olog client", e);
5360
}
54-
return oLogClient;
61+
return null;
5562
}
56-
5763
}

‎app/logbook/olog/client-es/src/main/java/org/phoebus/olog/es/api/OlogClient.java‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.phoebus.logbook.Attachment;
2020
import org.phoebus.logbook.LogClient;
2121
import org.phoebus.logbook.LogEntry;
22+
import org.phoebus.logbook.LogService;
2223
import org.phoebus.logbook.Logbook;
2324
import org.phoebus.logbook.LogbookException;
2425
import org.phoebus.logbook.Messages;
@@ -28,6 +29,8 @@
2829
import org.phoebus.olog.es.api.model.OlogLog;
2930
import org.phoebus.olog.es.api.model.OlogObjectMappers;
3031
import org.phoebus.olog.es.api.model.OlogSearchResult;
32+
import org.phoebus.security.store.SecureStore;
33+
import org.phoebus.security.tokens.ScopedAuthenticationToken;
3134

3235
import javax.net.ssl.SSLContext;
3336
import javax.ws.rs.core.MediaType;
@@ -145,8 +148,22 @@ public OlogClient create() {
145148
this.clientConfig = new DefaultClientConfig();
146149
}
147150
}
148-
this.username = ifNullReturnPreferenceValue(this.username, "username");
149-
this.password = ifNullReturnPreferenceValue(this.password, "password");
151+
// Check if cached credentials are available.
152+
ScopedAuthenticationToken scopedAuthenticationToken = null;
153+
try {
154+
SecureStore store = new SecureStore();
155+
scopedAuthenticationToken = store.getScopedAuthenticationToken(LogService.AUTHENTICATION_SCOPE);
156+
} catch (Exception e) {
157+
logger.log(Level.SEVERE, "Unable to query for scoped authentication token");
158+
}
159+
if(scopedAuthenticationToken != null){
160+
this.username = scopedAuthenticationToken.getUsername();
161+
this.password = scopedAuthenticationToken.getPassword();
162+
}
163+
else{
164+
this.username = ifNullReturnPreferenceValue(this.username, "username");
165+
this.password = ifNullReturnPreferenceValue(this.password, "password");
166+
}
150167
this.connectTimeoutAsString = ifNullReturnPreferenceValue(this.connectTimeoutAsString, "connectTimeout");
151168
int connectTimeout = 0;
152169
try {

‎app/logbook/olog/ui/doc/images/ReplyAnnotation.png‎

5.68 KB
Loading

‎app/logbook/olog/ui/doc/index.rst‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,11 +207,19 @@ feature user can choose to:
207207

208208
.. image:: images/ContextMenuLogEntryTable.png
209209

210-
Log entries that are contained in a log entry group are rendered with a grey background in the search result table view.
210+
Log entries that are contained in a log entry group are rendered with a "reply" icon in the search result table view:
211+
212+
.. image:: images/ReplyAnnotation.png
213+
211214
In the log entry view, the "Show/Hide Group" button (see screen shot above) can be used to show all log entries of a group sequentially,
212215
ordered on created date with oldest log entry on top. In this merged view attachments and properties are not shown.
213216
Clicking on a header in the merged view will show that log entry in full.
214217

218+
**NOTE**: To be able to group log entries user must be authenticated in one of the following manners:
219+
220+
* Use "credentials caching" through preference setting ``org.phoebus.logbook.olog.ui/save_credentials``. Once a log entry has been created, credentials will be reused when creating a group.
221+
* Use the Credentials Management app to sign in to the logbook context.
222+
215223
Limitations
216224
^^^^^^^^^^^
217225

‎app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/LogEntryTableViewController.java‎

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,15 @@
4242
import org.phoebus.framework.jobs.JobManager;
4343
import org.phoebus.logbook.LogClient;
4444
import org.phoebus.logbook.LogEntry;
45+
import org.phoebus.logbook.LogService;
4546
import org.phoebus.logbook.LogbookException;
47+
import org.phoebus.logbook.LogbookPreferences;
4648
import org.phoebus.logbook.SearchResult;
4749
import org.phoebus.logbook.olog.ui.query.OlogQuery;
4850
import org.phoebus.logbook.olog.ui.query.OlogQueryManager;
4951
import org.phoebus.olog.es.api.model.LogGroupProperty;
52+
import org.phoebus.security.store.SecureStore;
53+
import org.phoebus.security.tokens.ScopedAuthenticationToken;
5054
import org.phoebus.ui.dialog.DialogHelper;
5155
import org.phoebus.ui.dialog.ExceptionDetailsErrorDialog;
5256

@@ -123,6 +127,7 @@ public LogEntryTableViewController(LogClient logClient, OlogQueryManager ologQue
123127
private final SimpleIntegerProperty pageCountProperty = new SimpleIntegerProperty(0);
124128
private final OlogQueryManager ologQueryManager;
125129
private final ObservableList<OlogQuery> ologQueries = FXCollections.observableArrayList();
130+
private final SimpleBooleanProperty userHasSignedIn = new SimpleBooleanProperty(false);
126131

127132
private final SearchParameters searchParameters;
128133

@@ -145,7 +150,7 @@ public void initialize() {
145150

146151
groupSelectedEntries.disableProperty()
147152
.bind(Bindings.createBooleanBinding(() ->
148-
selectedLogEntries.size() < 2, selectedLogEntries));
153+
selectedLogEntries.size() < 2 || userHasSignedIn.not().get(), selectedLogEntries, userHasSignedIn));
149154
ContextMenu contextMenu = new ContextMenu();
150155

151156
MenuItem menuItemShowHideAll = new MenuItem(Messages.ShowHideDetails);
@@ -156,14 +161,23 @@ public void initialize() {
156161
});
157162

158163
contextMenu.getItems().addAll(groupSelectedEntries, menuItemShowHideAll);
164+
contextMenu.setOnShowing(e -> {
165+
try {
166+
SecureStore store = new SecureStore();
167+
ScopedAuthenticationToken scopedAuthenticationToken = store.getScopedAuthenticationToken(LogService.AUTHENTICATION_SCOPE);
168+
userHasSignedIn.set(scopedAuthenticationToken != null);
169+
} catch (Exception ex) {
170+
logger.log(Level.WARNING, "Secure Store file not found.", ex);
171+
}
172+
});
159173

160174
tableView.setContextMenu(contextMenu);
161175

162176
// The display table.
163177
tableView.getColumns().clear();
164178
tableView.setEditable(false);
165179
tableView.getSelectionModel().selectedItemProperty().addListener((observable, oldValue, newValue) -> {
166-
if(newValue != null && tableView.getSelectionModel().getSelectedItems().size() == 1){
180+
if (newValue != null && tableView.getSelectionModel().getSelectedItems().size() == 1) {
167181
selectedLogEntry = newValue.getLogEntry();
168182
logEntryDisplayController.setLogEntry(newValue.getLogEntry());
169183
}
@@ -177,7 +191,7 @@ public void initialize() {
177191

178192
descriptionCol = new TableColumn<>();
179193
descriptionCol.setMaxWidth(1f * Integer.MAX_VALUE * 100);
180-
descriptionCol.setCellValueFactory(col -> new SimpleObjectProperty(col.getValue()));
194+
descriptionCol.setCellValueFactory(col -> new SimpleObjectProperty<>(col.getValue()));
181195
descriptionCol.setCellFactory(col -> new TableCell<>() {
182196
private final Node graphic;
183197
private final PseudoClass childlessTopLevel =
@@ -224,7 +238,7 @@ public void updateItem(TableViewListItem logEntry, boolean empty) {
224238
// This is to accept numerical input only, and at most 3 digits (maximizing search to 999 hits).
225239
pageSizeTextField.textProperty().addListener((observable, oldValue, newValue) -> {
226240
if (DIGIT_PATTERN.matcher(newValue).matches()) {
227-
if ("".equals(newValue)) {
241+
if ("" .equals(newValue)) {
228242
pageSizeProperty.set(LogbookUIPreferences.search_result_page_size);
229243
} else if (newValue.length() > 3) {
230244
pageSizeTextField.setText(oldValue);
@@ -300,7 +314,7 @@ public void resize() {
300314
*/
301315
public void search() {
302316
// In case the page size text field is empty, or the value is zero, set the page size to the default
303-
if ("".equals(pageSizeTextField.getText()) || Integer.parseInt(pageSizeTextField.getText()) == 0) {
317+
if ("" .equals(pageSizeTextField.getText()) || Integer.parseInt(pageSizeTextField.getText()) == 0) {
304318
pageSizeTextField.setText(Integer.toString(LogbookUIPreferences.search_result_page_size));
305319
}
306320

@@ -366,9 +380,9 @@ private void refresh() {
366380
tableView.setItems(logsList);
367381
// This will ensure that if an entry was selected, it stays selected after the list has been
368382
// updated from the search result, even if it is empty.
369-
if(selectedLogEntry != null){
370-
for(TableViewListItem item : tableView.getItems()){
371-
if(item.getLogEntry().getId().equals(selectedLogEntry.getId())){
383+
if (selectedLogEntry != null) {
384+
for (TableViewListItem item : tableView.getItems()) {
385+
if (item.getLogEntry().getId().equals(selectedLogEntry.getId())) {
372386
Platform.runLater(() -> tableView.getSelectionModel().select(item));
373387
break;
374388
}
@@ -382,7 +396,8 @@ private void createLogEntryGroup() {
382396
selectedLogEntries.stream().map(LogEntry::getId).collect(Collectors.toList());
383397
JobManager.schedule("Group log entries", monitor -> {
384398
try {
385-
client.groupLogEntries(logEntryIds);
399+
LogClient logClient = LogService.getInstance().getLogFactories().get(LogbookPreferences.logbook_factory).getLogClient();
400+
logClient.groupLogEntries(logEntryIds);
386401
search();
387402
} catch (LogbookException e) {
388403
logger.log(Level.INFO, "Unable to create log entry group from selection");
@@ -483,15 +498,15 @@ public void setShowDetails(boolean show) {
483498
}
484499
}
485500

486-
public void setShowDetails(boolean show){
501+
public void setShowDetails(boolean show) {
487502
showDetails.set(show);
488503
}
489504

490-
public boolean getShowDetails(){
505+
public boolean getShowDetails() {
491506
return showDetails.get();
492507
}
493508

494-
public void showHelp(){
509+
public void showHelp() {
495510
new HelpViewer(LogbookUIPreferences.search_help).show();
496511
}
497512
}

‎app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/LogbookSearchController.java‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public abstract class LogbookSearchController {
2727

2828
private Job logbookSearchJob;
2929
protected LogClient client;
30-
private ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
30+
private final ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
3131
private ScheduledFuture<?> runningTask;
3232
protected final SimpleBooleanProperty searchInProgress = new SimpleBooleanProperty(false);
3333
private static final int SEARCH_JOB_INTERVAL = 30; // seconds
@@ -62,15 +62,13 @@ public void search(Map<String, String> searchParams, final Consumer<SearchResult
6262
*/
6363
public void periodicSearch(Map<String, String> searchParams, final Consumer<SearchResult> resultHandler) {
6464
cancelPeriodSearch();
65-
runningTask = executor.scheduleAtFixedRate(() -> {
66-
logbookSearchJob = LogbookSearchJob.submit(this.client,
67-
searchParams,
68-
resultHandler,
69-
(url, ex) -> {
70-
searchInProgress.set(false);
71-
cancelPeriodSearch();
72-
});
73-
}, 0, SEARCH_JOB_INTERVAL, TimeUnit.SECONDS);
65+
runningTask = executor.scheduleAtFixedRate(() -> logbookSearchJob = LogbookSearchJob.submit(this.client,
66+
searchParams,
67+
resultHandler,
68+
(url, ex) -> {
69+
searchInProgress.set(false);
70+
cancelPeriodSearch();
71+
}), SEARCH_JOB_INTERVAL, SEARCH_JOB_INTERVAL, TimeUnit.SECONDS);
7472
}
7573

7674
@Deprecated

0 commit comments

Comments
 (0)