Skip to content

Commit 1bbba55

Browse files
stroomdev66stroomdev66
committed
Merge branch '7.5' of github.com:gchq/stroom into 7.6
2 parents 8631157 + 8b28862 commit 1bbba55

File tree

9 files changed

+204
-87
lines changed

9 files changed

+204
-87
lines changed

‎.github/workflows/build_and_release.yml

+4
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ jobs:
9696
# https://github.com/gchq/stroom/settings/secrets/actions
9797
DOCKER_USERNAME: ${{ secrets.DOCKER_USERNAME }}
9898
DOCKER_PASSWORD: ${{ secrets.DOCKER_PASSWORD }}
99+
#PAT_FOR_PUBLIC_REPOS: ${{ secrets.PAT_FOR_PUBLIC_REPOS }}
100+
# The special auto created token, used for authenticating
101+
# downloads prevent rate limiting
102+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
99103
run: |
100104
pushd "${BUILD_DIR}" > /dev/null
101105
echo -e "${GREEN}Running ${BLUE}ci_build.sh${NC}"

‎stroom-core-client/src/main/java/stroom/dispatch/client/ExportFileCompleteUtil.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ private static String getMessage(final ResourceGeneration result) {
7676
private static void download(final LocationManager locationManager, final ResourceGeneration result) {
7777
// Change the browser location to download the zip
7878
// file.
79-
locationManager.replace(GWT.getHostPageBaseURL() + "resourcestore/" +
80-
result.getResourceKey().getName() + "?UUID=" + result.getResourceKey().getKey());
79+
locationManager.replace(GWT.getHostPageBaseURL() +
80+
"resourcestore/?uuid=" +
81+
result.getResourceKey().getKey());
8182
}
8283
}

‎stroom-data/stroom-data-store-impl/src/main/java/stroom/data/store/impl/ImportFileServlet.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,8 @@ protected void doPost(final HttpServletRequest request, final HttpServletRespons
9898
}
9999

100100
propertyMap.setSuccess(true);
101-
resourceKey.write(propertyMap);
101+
propertyMap.put(ResourceKey.NAME, fileName);
102+
propertyMap.put(ResourceKey.KEY, resourceKey.getKey());
102103
fileItem.delete();
103104

104105
} catch (final RuntimeException e) {

‎stroom-resource/stroom-resource-api/src/main/java/stroom/resource/api/ResourceStore.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
public interface ResourceStore {
2828

2929
/**
30-
* Create a temporary file and give it a string key.
30+
* Create a temporary file and give it a UUID.
3131
*/
3232
ResourceKey createTempFile(final String name);
3333

‎stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/ResourceStoreImpl.java

+95-31
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
import stroom.task.api.TaskContextFactory;
2121
import stroom.util.io.FileUtil;
2222
import stroom.util.io.TempDirProvider;
23+
import stroom.util.logging.LambdaLogger;
24+
import stroom.util.logging.LambdaLoggerFactory;
2325
import stroom.util.shared.ResourceKey;
2426

2527
import jakarta.inject.Inject;
@@ -29,23 +31,24 @@
2931
import java.io.UncheckedIOException;
3032
import java.nio.file.Files;
3133
import java.nio.file.Path;
32-
import java.nio.file.Paths;
33-
import java.util.HashSet;
34-
import java.util.Set;
34+
import java.time.Instant;
35+
import java.util.Map;
36+
import java.util.UUID;
37+
import java.util.concurrent.ConcurrentHashMap;
3538

3639
/**
37-
* Simple Store that gives you 1 hour to use your temp file and then it deletes
38-
* it.
40+
* Simple Store that gives you 1 hour to use your temp file before it deletes it.
3941
*/
4042
@Singleton
4143
public class ResourceStoreImpl implements ResourceStore {
4244

45+
private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(ResourceStoreImpl.class);
46+
4347
private final TempDirProvider tempDirProvider;
4448
private final TaskContextFactory taskContextFactory;
49+
private final Map<String, ResourceItem> currentFiles = new ConcurrentHashMap<>();
4550

46-
private Set<ResourceKey> currentFiles = new HashSet<>();
47-
private Set<ResourceKey> oldFiles = new HashSet<>();
48-
private long sequence;
51+
private volatile Instant lastCleanupTime;
4952

5053
@Inject
5154
public ResourceStoreImpl(final TempDirProvider tempDirProvider,
@@ -73,47 +76,108 @@ void shutdown() {
7376
}
7477

7578
@Override
76-
public synchronized ResourceKey createTempFile(final String name) {
77-
final String fileName = FileUtil.getCanonicalPath(getTempDir().resolve((sequence++) + name));
78-
final ResourceKey resourceKey = new ResourceKey(name, fileName);
79-
currentFiles.add(resourceKey);
80-
79+
public ResourceKey createTempFile(final String name) {
80+
final String uuid = UUID.randomUUID().toString();
81+
final Path path = getTempDir().resolve(uuid);
82+
final ResourceKey resourceKey = new ResourceKey(uuid, name);
83+
final ResourceItem resourceItem = new ResourceItem(resourceKey, path, Instant.now());
84+
currentFiles.put(uuid, resourceItem);
8185
return resourceKey;
8286
}
8387

8488
@Override
85-
public synchronized void deleteTempFile(final ResourceKey resourceKey) {
86-
currentFiles.remove(resourceKey);
87-
oldFiles.remove(resourceKey);
88-
final Path file = Paths.get(resourceKey.getKey());
89-
try {
90-
Files.deleteIfExists(file);
91-
} catch (final IOException e) {
92-
throw new UncheckedIOException(e);
89+
public void deleteTempFile(final ResourceKey resourceKey) {
90+
final ResourceItem resourceItem = currentFiles.remove(resourceKey.getKey());
91+
if (resourceItem != null) {
92+
final Path file = resourceItem.getPath();
93+
try {
94+
Files.deleteIfExists(file);
95+
} catch (final IOException e) {
96+
throw new UncheckedIOException(e);
97+
}
9398
}
9499
}
95100

96101
@Override
97-
public synchronized Path getTempFile(final ResourceKey resourceKey) {
102+
public Path getTempFile(final ResourceKey resourceKey) {
98103
// File gone !
99-
if (!currentFiles.contains(resourceKey) && !oldFiles.contains(resourceKey)) {
104+
final ResourceItem resourceItem = currentFiles.get(resourceKey.getKey());
105+
if (resourceItem == null) {
100106
return null;
101107
}
102-
return Paths.get(resourceKey.getKey());
108+
resourceItem.setLastAccessTime(Instant.now());
109+
return resourceItem.getPath();
103110
}
104111

105112
void execute() {
106113
taskContextFactory.current().info(() -> "Deleting temp files");
107-
flipStore();
114+
cleanup();
108115
}
109116

110117
/**
111-
* Move the current files to the old files deleting the old ones.
118+
* Delete files that haven't been accessed since the last cleanup.
119+
* This allows us to choose the cleanup frequency.
112120
*/
113-
private synchronized void flipStore() {
114-
final Set<ResourceKey> clonedOldFiles = new HashSet<>(oldFiles);
115-
clonedOldFiles.forEach(this::deleteTempFile);
116-
oldFiles = currentFiles;
117-
currentFiles = new HashSet<>();
121+
private synchronized void cleanup() {
122+
if (lastCleanupTime != null) {
123+
// Delete anything that hasn't been accessed since we were last asked to cleanup.
124+
currentFiles.values().forEach(resourceItem -> {
125+
try {
126+
if (resourceItem.getLastAccessTime().isBefore(lastCleanupTime)) {
127+
deleteTempFile(resourceItem.getResourceKey());
128+
}
129+
} catch (final RuntimeException e) {
130+
LOGGER.error(e::getMessage, e);
131+
}
132+
});
133+
}
134+
lastCleanupTime = Instant.now();
135+
}
136+
137+
private static class ResourceItem {
138+
139+
private final ResourceKey resourceKey;
140+
private final Path path;
141+
private final Instant createTime;
142+
private volatile Instant lastAccessTime;
143+
144+
public ResourceItem(final ResourceKey resourceKey,
145+
final Path path,
146+
final Instant createTime) {
147+
this.resourceKey = resourceKey;
148+
this.path = path;
149+
this.createTime = createTime;
150+
this.lastAccessTime = createTime;
151+
}
152+
153+
public ResourceKey getResourceKey() {
154+
return resourceKey;
155+
}
156+
157+
public Path getPath() {
158+
return path;
159+
}
160+
161+
public Instant getCreateTime() {
162+
return createTime;
163+
}
164+
165+
public Instant getLastAccessTime() {
166+
return lastAccessTime;
167+
}
168+
169+
public void setLastAccessTime(final Instant lastAccessTime) {
170+
this.lastAccessTime = lastAccessTime;
171+
}
172+
173+
@Override
174+
public String toString() {
175+
return "ResourceItem{" +
176+
"resourceKey=" + resourceKey +
177+
", path=" + path +
178+
", createTime=" + createTime +
179+
", lastAccessTime=" + lastAccessTime +
180+
'}';
181+
}
118182
}
119183
}

‎stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceStoreImpl.java

+29-25
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package stroom.resource.impl;
1818

1919
import stroom.resource.api.ResourceStore;
20-
import stroom.util.io.FileUtil;
2120
import stroom.util.servlet.HttpServletRequestHolder;
2221
import stroom.util.shared.IsServlet;
2322
import stroom.util.shared.ResourceKey;
@@ -40,11 +39,10 @@
4039
*/
4140
public class SessionResourceStoreImpl extends HttpServlet implements ResourceStore, IsServlet {
4241

43-
private static final long serialVersionUID = -4533441835216235920L;
4442
private static final Set<String> PATH_SPECS = Set.of("/resourcestore/*");
45-
private static final String UUID_ARG = "UUID";
43+
private static final String UUID_ARG = "uuid";
4644

47-
private final ResourceStore resourceStore;
45+
private final ResourceStoreImpl resourceStore;
4846
private final HttpServletRequestHolder httpServletRequestHolder;
4947

5048
@Inject
@@ -57,29 +55,33 @@ public class SessionResourceStoreImpl extends HttpServlet implements ResourceSto
5755
@Override
5856
public ResourceKey createTempFile(final String name) {
5957
final ResourceKey key = resourceStore.createTempFile(name);
60-
final ResourceKey sessionKey = new ResourceKey(name, UUID.randomUUID().toString());
58+
final ResourceKey sessionKey = new ResourceKey(UUID.randomUUID().toString(), name);
6159

6260
getMap().put(sessionKey, key);
6361

6462
return sessionKey;
6563
}
6664

6765
@Override
68-
public void deleteTempFile(final ResourceKey key) {
69-
final ResourceKey realKey = getMap().remove(key);
66+
public Path getTempFile(final ResourceKey key) {
67+
final ResourceKey realKey = getRealKey(key);
7068
if (realKey == null) {
71-
return;
69+
return null;
7270
}
73-
resourceStore.deleteTempFile(realKey);
71+
return resourceStore.getTempFile(realKey);
7472
}
7573

7674
@Override
77-
public Path getTempFile(final ResourceKey key) {
78-
final ResourceKey realKey = getMap().get(key);
75+
public void deleteTempFile(final ResourceKey key) {
76+
final ResourceKey realKey = getRealKey(key);
7977
if (realKey == null) {
80-
return null;
78+
return;
8179
}
82-
return resourceStore.getTempFile(getMap().get(key));
80+
resourceStore.deleteTempFile(realKey);
81+
}
82+
83+
private ResourceKey getRealKey(final ResourceKey key) {
84+
return getMap().get(key);
8385
}
8486

8587
private ResourceMap getMap() {
@@ -114,20 +116,22 @@ protected void doGet(final HttpServletRequest req, final HttpServletResponse res
114116
final String uuid = req.getParameter(UUID_ARG);
115117
boolean found = false;
116118
if (uuid != null) {
117-
final ResourceKey resourceKey = new ResourceKey(null, uuid);
118-
try {
119-
final Path file = getTempFile(resourceKey);
120-
if (file != null && Files.isRegularFile(file)) {
121-
if (FileUtil.getCanonicalPath(file).toLowerCase().endsWith(".zip")) {
122-
resp.setContentType("application/zip");
123-
} else {
124-
resp.setContentType("application/octet-stream");
119+
final ResourceKey resourceKey = getRealKey(new ResourceKey(uuid, null));
120+
if (resourceKey != null) {
121+
try {
122+
final Path file = resourceStore.getTempFile(resourceKey);
123+
if (file != null && Files.isRegularFile(file)) {
124+
if (resourceKey.getName().toLowerCase().endsWith(".zip")) {
125+
resp.setContentType("application/zip");
126+
} else {
127+
resp.setContentType("application/octet-stream");
128+
}
129+
resp.getOutputStream().write(Files.readAllBytes(file));
130+
found = true;
125131
}
126-
resp.getOutputStream().write(Files.readAllBytes(file));
127-
found = true;
132+
} finally {
133+
deleteTempFile(resourceKey);
128134
}
129-
} finally {
130-
deleteTempFile(resourceKey);
131135
}
132136
}
133137

‎stroom-resource/stroom-resource-impl/src/test/java/stroom/resource/impl/TestResourceStore.java

+28-6
Original file line numberDiff line numberDiff line change
@@ -39,25 +39,47 @@ void testSimple() throws IOException {
3939
resourceStore.execute();
4040

4141
final ResourceKey key1 = resourceStore.createTempFile("TestResourceStore1.dat");
42-
assertThat(key1.toString().endsWith("TestResourceStore1.dat")).isTrue();
42+
assertThat(key1.getName()).isEqualTo("TestResourceStore1.dat");
4343

4444
final ResourceKey key2 = resourceStore.createTempFile("TestResourceStore2.dat");
45-
assertThat(key2.toString().endsWith("TestResourceStore2.dat")).isTrue();
45+
assertThat(key2.getName()).isEqualTo("TestResourceStore2.dat");
4646

4747
Files.createFile(resourceStore.getTempFile(key1));
4848
Files.createFile(resourceStore.getTempFile(key2));
4949

5050
assertThat(Files.isRegularFile(resourceStore.getTempFile(key1))).isTrue();
5151
assertThat(Files.isRegularFile(resourceStore.getTempFile(key2))).isTrue();
5252

53-
// Roll to Old
53+
// Cleanup
5454
resourceStore.execute();
55-
final Path file1 = resourceStore.getTempFile(key1);
55+
Path file1 = resourceStore.getTempFile(key1);
5656
assertThat(Files.isRegularFile(file1)).isTrue();
57-
final Path file2 = resourceStore.getTempFile(key2);
57+
Path file2 = resourceStore.getTempFile(key2);
5858
assertThat(Files.isRegularFile(file2)).isTrue();
5959

60-
// Roll to Delete
60+
// Cleanup
61+
resourceStore.execute();
62+
63+
// Files should still exist as we have accessed them since last roll.
64+
file1 = resourceStore.getTempFile(key1);
65+
assertThat(Files.isRegularFile(file1)).isTrue();
66+
file2 = resourceStore.getTempFile(key2);
67+
assertThat(Files.isRegularFile(file2)).isTrue();
68+
69+
// Cleanup
70+
resourceStore.execute();
71+
file1 = resourceStore.getTempFile(key1);
72+
assertThat(Files.isRegularFile(file1)).isTrue();
73+
resourceStore.execute();
74+
75+
// We should have deleted file2 as we didn't access since last cleanup.
76+
file1 = resourceStore.getTempFile(key1);
77+
assertThat(Files.isRegularFile(file1)).isTrue();
78+
assertThat(resourceStore.getTempFile(key2)).isNull();
79+
assertThat(Files.isRegularFile(file2)).isFalse();
80+
81+
// Now delete everything.
82+
resourceStore.execute();
6183
resourceStore.execute();
6284
assertThat(resourceStore.getTempFile(key1)).isNull();
6385
assertThat(Files.isRegularFile(file1)).isFalse();

0 commit comments

Comments
 (0)