Skip to content

Commit 18032ef

Browse files
at055612at055612
authored
Merge pull request #4193 from gchq/gh-4192-folder-info-npe
PR for #4192 - NPE when selecting Info in the explorer context menu for a Folder
2 parents b3588c4 + 127be43 commit 18032ef

File tree

7 files changed

+257
-34
lines changed

7 files changed

+257
-34
lines changed

‎stroom-app/src/test/java/stroom/db/migration/TestListDbMigrations.java

+3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import com.google.common.base.Strings;
99
import jakarta.validation.constraints.NotNull;
1010
import org.junit.jupiter.api.Test;
11+
import org.junit.jupiter.api.parallel.Isolated;
1112
import org.slf4j.Logger;
1213
import org.slf4j.LoggerFactory;
1314

@@ -31,6 +32,8 @@
3132
import static stroom.util.ConsoleColour.RED;
3233
import static stroom.util.ConsoleColour.YELLOW;
3334

35+
@Isolated // This is walking the file tree so doesn't like it if another test is mutating
36+
// the file tree at the same time.
3437
public class TestListDbMigrations {
3538

3639
private static final Logger LOGGER = LoggerFactory.getLogger(TestListDbMigrations.class);

‎stroom-explorer/stroom-explorer-impl/src/main/java/stroom/explorer/impl/DocRefInfoCache.java

+61-23
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@
2020
import stroom.cache.api.LoadingStroomCache;
2121
import stroom.docref.DocRef;
2222
import stroom.docref.DocRefInfo;
23-
import stroom.docstore.api.DocumentActionHandler;
2423
import stroom.docstore.api.DocumentActionHandlers;
2524
import stroom.docstore.api.DocumentNotFoundException;
2625
import stroom.security.api.SecurityContext;
26+
import stroom.util.NullSafe;
2727
import stroom.util.entityevent.EntityAction;
2828
import stroom.util.entityevent.EntityEvent;
2929
import stroom.util.entityevent.EntityEventHandler;
@@ -35,8 +35,10 @@
3535
import org.slf4j.Logger;
3636
import org.slf4j.LoggerFactory;
3737

38+
import java.util.HashSet;
3839
import java.util.Objects;
3940
import java.util.Optional;
41+
import java.util.Set;
4042

4143
@Singleton
4244
@EntityEventHandler(action = {
@@ -53,15 +55,18 @@ class DocRefInfoCache implements EntityEvent.Handler, Clearable {
5355
private final SecurityContext securityContext;
5456
// Provider to avoid circular guice dependency issue
5557
private final Provider<DocumentActionHandlers> documentActionHandlersProvider;
58+
private final ExplorerActionHandlers explorerActionHandlers;
5659

5760

5861
@Inject
5962
DocRefInfoCache(final CacheManager cacheManager,
6063
final Provider<ExplorerConfig> explorerConfigProvider,
6164
final SecurityContext securityContext,
62-
final Provider<DocumentActionHandlers> documentActionHandlersProvider) {
65+
final Provider<DocumentActionHandlers> documentActionHandlersProvider,
66+
final ExplorerActionHandlers explorerActionHandlers) {
6367
this.securityContext = securityContext;
6468
this.documentActionHandlersProvider = documentActionHandlersProvider;
69+
this.explorerActionHandlers = explorerActionHandlers;
6570

6671
cache = cacheManager.createLoadingCache(
6772
CACHE_NAME,
@@ -76,28 +81,9 @@ private Optional<DocRefInfo> loadDocRefInfo(final DocRefCacheKey docRefCacheKey)
7681
docRefInfo = securityContext.asProcessingUserResult(() -> {
7782
final DocRef docRef = docRefCacheKey.getDocRef();
7883
if (docRef.getType() != null) {
79-
final DocumentActionHandler<?> handler = documentActionHandlersProvider.get()
80-
.getHandler(docRef.getType());
81-
if (handler == null) {
82-
return null;
83-
}
84-
return handler.info(docRef.getUuid());
84+
return getDocRefInfoWithType(docRef);
8585
} else {
86-
final String uuid = docRef.getUuid();
87-
// No type so need to check all handlers and return the one that has it.
88-
// Hopefully next time it will still be in the cache so this won't be needed
89-
final Optional<DocRefInfo> optInfo = documentActionHandlersProvider.get()
90-
.stream()
91-
.map(handler -> {
92-
try {
93-
return handler.info(uuid);
94-
} catch (DocumentNotFoundException e) {
95-
return null;
96-
}
97-
})
98-
.filter(Objects::nonNull)
99-
.findAny();
100-
return optInfo.orElse(null);
86+
return getDocRefInfoWithoutType(docRef);
10187
}
10288
});
10389
} catch (final RuntimeException e) {
@@ -106,6 +92,58 @@ private Optional<DocRefInfo> loadDocRefInfo(final DocRefCacheKey docRefCacheKey)
10692
return Optional.ofNullable(docRefInfo);
10793
}
10894

95+
private DocRefInfo getDocRefInfoWithoutType(final DocRef docRef) {
96+
final String uuid = docRef.getUuid();
97+
// No type so need to check all handlers and return the one that has it.
98+
// Hopefully next time it will still be in the cache so this won't be needed
99+
final Set<String> typesChecked = new HashSet<>();
100+
final Optional<DocRefInfo> optInfo = documentActionHandlersProvider.get()
101+
.stream()
102+
.map(handler -> {
103+
typesChecked.add(handler.getType());
104+
try {
105+
return handler.info(uuid);
106+
} catch (DocumentNotFoundException e) {
107+
return null;
108+
}
109+
})
110+
.filter(Objects::nonNull)
111+
.findAny();
112+
113+
// Folder is not a DocumentActionHandler so check in ExplorerActionHandlers
114+
return optInfo
115+
.or(() ->
116+
explorerActionHandlers.stream()
117+
.filter(handler ->
118+
!typesChecked.contains(handler.getDocumentType().getType()))
119+
.map(handler -> {
120+
try {
121+
return handler.info(uuid);
122+
} catch (DocumentNotFoundException e) {
123+
return null;
124+
}
125+
})
126+
.filter(Objects::nonNull)
127+
.findAny())
128+
.orElse(null);
129+
}
130+
131+
private DocRefInfo getDocRefInfoWithType(final DocRef docRef) {
132+
final String type = docRef.getType();
133+
final String uuid = docRef.getUuid();
134+
135+
DocRefInfo docRefInfo = NullSafe.get(
136+
documentActionHandlersProvider.get().getHandler(type),
137+
handler -> handler.info(uuid));
138+
139+
if (docRefInfo == null) {
140+
docRefInfo = NullSafe.get(
141+
explorerActionHandlers.getHandler(type),
142+
handler -> handler.info(uuid));
143+
}
144+
return docRefInfo;
145+
}
146+
109147
Optional<DocRefInfo> get(final DocRef docRef) {
110148
return cache.get(new DocRefCacheKey(docRef));
111149
}

‎stroom-explorer/stroom-explorer-impl/src/main/java/stroom/explorer/impl/ExplorerActionHandlers.java

+10
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,11 @@
2727
import java.util.Comparator;
2828
import java.util.List;
2929
import java.util.Map;
30+
import java.util.Objects;
3031
import java.util.Set;
3132
import java.util.concurrent.ConcurrentHashMap;
3233
import java.util.function.BiConsumer;
34+
import java.util.stream.Stream;
3335

3436
@Singleton
3537
class ExplorerActionHandlers {
@@ -78,6 +80,14 @@ public void forEach(final BiConsumer<String, ExplorerActionHandler> consumer) {
7880
});
7981
}
8082

83+
public Stream<ExplorerActionHandler> stream() {
84+
return getHandlers()
85+
.allHandlers
86+
.values()
87+
.stream()
88+
.filter(Objects::nonNull);
89+
}
90+
8191

8292
// --------------------------------------------------------------------------------
8393

‎stroom-explorer/stroom-explorer-impl/src/main/java/stroom/explorer/impl/FolderExplorerActionHandler.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import stroom.docref.DocRef;
66
import stroom.docref.DocRefInfo;
77
import stroom.docref.StringMatch;
8+
import stroom.docstore.api.DocumentNotFoundException;
89
import stroom.docstore.api.UniqueNameUtil;
910
import stroom.explorer.api.ExplorerActionHandler;
1011
import stroom.explorer.shared.DocumentType;
@@ -118,7 +119,9 @@ public void deleteDocument(final String uuid) {
118119
public DocRefInfo info(final String uuid) {
119120
final ExplorerTreeNode explorerTreeNode = explorerTreeDao.findByUUID(uuid);
120121
if (explorerTreeNode == null) {
121-
throw new RuntimeException("Unable to find tree node to get info");
122+
throw new DocumentNotFoundException(DocRef.builder()
123+
.uuid(uuid)
124+
.build());
122125
}
123126

124127
if (!securityContext.hasDocumentPermission(uuid, DocumentPermissionNames.READ)) {

0 commit comments

Comments
 (0)