Skip to content

Commit 2ffcc5d

Browse files
stroomdev66stroomdev66
authored
Merge pull request #4816 from gchq/gh-4812
#4812 Fix data download issue
2 parents e7c7d83 + 9585239 commit 2ffcc5d

File tree

19 files changed

+145
-99
lines changed

19 files changed

+145
-99
lines changed

Diff for: ‎stroom-app/src/test/java/stroom/importexport/TestImportExportDashboards.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,8 @@ private void test(final boolean skipVisCreation, final boolean skipVisExport, fi
286286

287287
// Load the dashboard.
288288
final VisualisationDoc loadedVisualisation = first(visualisationStore);
289-
final DocRef loadedPipeline = pipelineStore.list().get(0);
290-
final DocRef loadedIndex = indexStore.list().get(0);
289+
final DocRef loadedPipeline = pipelineStore.list().getFirst();
290+
final DocRef loadedIndex = indexStore.list().getFirst();
291291
final DictionaryDoc loadedDictionary = first(dictionaryStore);
292292
final DashboardDoc loadedDashboard = first(dashboardStore);
293293
final List<ComponentConfig> loadedComponents = loadedDashboard.getDashboardConfig().getComponents();
@@ -318,7 +318,7 @@ private void test(final boolean skipVisCreation, final boolean skipVisExport, fi
318318

319319
private <T extends Doc> T first(final DocumentStore<T> store) {
320320
final Set<DocRef> set = store.listDocuments();
321-
if (set != null && set.size() > 0) {
321+
if (set != null && !set.isEmpty()) {
322322
return store.readDocument(set.iterator().next());
323323
}
324324
return null;

Diff for: ‎stroom-aws/stroom-aws-s3-impl/src/main/java/stroom/aws/s3/impl/S3ConfigResourceImpl.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,10 @@ public ResourceGeneration download(final DocRef s3ConfigDocRef) {
6969
}
7070

7171
final ResourceKey resourceKey = resourceStoreProvider.get().createTempFile("s3Config.properties");
72-
final Path file = resourceStoreProvider.get().getTempFile(resourceKey);
72+
final Path tempFile = resourceStoreProvider.get().getTempFile(resourceKey);
7373
try {
74-
Files.writeString(file, s3ConfigDoc.getData(), StreamUtil.DEFAULT_CHARSET);
75-
} catch (IOException e) {
74+
Files.writeString(tempFile, s3ConfigDoc.getData(), StreamUtil.DEFAULT_CHARSET);
75+
} catch (final IOException e) {
7676
LOGGER.error("Unable to download S3Config", e);
7777
throw new UncheckedIOException(e);
7878
}

Diff for: ‎stroom-dashboard/stroom-dashboard-impl/src/main/java/stroom/dashboard/impl/DashboardServiceImpl.java

+9-19
Original file line numberDiff line numberDiff line change
@@ -226,10 +226,8 @@ public ResourceGeneration downloadQuery(final DashboardSearchRequest request) {
226226
String fileName = getQueryFileName(request);
227227

228228
final ResourceKey resourceKey = resourceStore.createTempFile(fileName);
229-
final Path outputFile = resourceStore.getTempFile(resourceKey);
230-
231-
JsonUtil.writeValue(outputFile, apiSearchRequest);
232-
229+
final Path tempFile = resourceStore.getTempFile(resourceKey);
230+
JsonUtil.writeValue(tempFile, apiSearchRequest);
233231
return new ResourceGeneration(resourceKey, new ArrayList<>());
234232
} catch (final RuntimeException e) {
235233
throw EntityServiceExceptionUtil.create(e);
@@ -280,28 +278,20 @@ public ResourceGeneration downloadSearchResults(final DownloadSearchResultsReque
280278
// Import file.
281279
final String fileName = getResultsFilename(request);
282280
resourceKey = resourceStore.createTempFile(fileName);
283-
final Path file = resourceStore.getTempFile(resourceKey);
281+
final Path tempFile = resourceStore.getTempFile(resourceKey);
284282

285283
final FormatterFactory formatterFactory =
286284
new FormatterFactory(searchRequest.getDateTimeSettings());
287285

288286
// Start target
289-
try (final OutputStream outputStream = new BufferedOutputStream(Files.newOutputStream(file))) {
290-
SearchResultWriter.Target target = null;
287+
try (final OutputStream outputStream = new BufferedOutputStream(Files.newOutputStream(tempFile))) {
288+
final SearchResultWriter.Target target = switch (request.getFileType()) {
289+
case CSV -> new DelimitedTarget(outputStream, ",");
290+
case TSV -> new DelimitedTarget(outputStream, "\t");
291+
case EXCEL -> new ExcelTarget(outputStream, searchRequest.getDateTimeSettings());
292+
};
291293

292294
// Write delimited file.
293-
switch (request.getFileType()) {
294-
case CSV:
295-
target = new DelimitedTarget(outputStream, ",");
296-
break;
297-
case TSV:
298-
target = new DelimitedTarget(outputStream, "\t");
299-
break;
300-
case EXCEL:
301-
target = new ExcelTarget(outputStream, searchRequest.getDateTimeSettings());
302-
break;
303-
}
304-
305295
try {
306296
target.start();
307297

Diff for: ‎stroom-data/stroom-data-store-impl/build.gradle

+1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ dependencies {
3030
implementation project(':stroom-docref')
3131
implementation libs.eventLogging
3232

33+
implementation libs.commons.compress
3334
implementation libs.commons.fileupload
3435
implementation libs.jackson.annotations
3536
implementation libs.jakarta.servlet.api

Diff for: ‎stroom-data/stroom-data-store-impl/src/main/java/stroom/data/store/impl/DataDownloadResourceImpl.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public Response downloadZip(final FindMetaCriteria criteria) {
6868
"Meta",
6969
criteria))
7070
.addFile(File.builder()
71-
.withName(tempFile.getFileName().toString())
71+
.withName(resourceKey.getName())
7272
.withSize(BigInteger.valueOf(Files.size(tempFile)))
7373
.build())
7474
.build())
@@ -85,12 +85,11 @@ public Response downloadZip(final FindMetaCriteria criteria) {
8585

8686
final Response response = Response
8787
.ok(streamingOutput, MediaType.APPLICATION_OCTET_STREAM)
88-
.header("Content-Disposition", "attachment; filename=\"" +
89-
tempFile.getFileName().toString() + "\"")
88+
.header("Content-Disposition", "attachment; filename=\"" + resourceKey.getName() + "\"")
9089
.build();
9190

9291
return ComplexLoggedOutcome.success(response, exportEventAction);
93-
} catch (IOException e) {
92+
} catch (final IOException e) {
9493
throw EntityServiceExceptionUtil.create(e);
9594
}
9695
})

Diff for: ‎stroom-data/stroom-data-store-impl/src/main/java/stroom/data/store/impl/DataDownloadTaskHandler.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public DataDownloadResult downloadData(final TaskContext taskContext,
9494
return securityContext.secureResult(() -> {
9595
final List<Meta> list = metaService.find(criteria).getValues();
9696
final DataDownloadResult result = new DataDownloadResult();
97-
if (list.size() == 0) {
97+
if (list.isEmpty()) {
9898
return result;
9999
}
100100

Diff for: ‎stroom-data/stroom-data-store-impl/src/main/java/stroom/data/store/impl/DataServiceImpl.java

+63-34
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,9 @@
5454
import stroom.ui.config.shared.SourceConfig;
5555
import stroom.util.NullSafe;
5656
import stroom.util.date.DateUtil;
57+
import stroom.util.io.FileUtil;
5758
import stroom.util.io.StreamUtil;
59+
import stroom.util.io.TempDirProvider;
5860
import stroom.util.logging.LambdaLogger;
5961
import stroom.util.logging.LambdaLoggerFactory;
6062
import stroom.util.logging.LogUtil;
@@ -65,18 +67,23 @@
6567
import stroom.util.shared.ResourceGeneration;
6668
import stroom.util.shared.ResourceKey;
6769
import stroom.util.shared.ResultPage;
70+
import stroom.util.zip.ZipUtil;
6871

6972
import jakarta.inject.Inject;
7073
import jakarta.inject.Provider;
7174

75+
import java.io.IOException;
76+
import java.nio.file.Files;
7277
import java.nio.file.Path;
7378
import java.text.NumberFormat;
7479
import java.util.ArrayList;
7580
import java.util.Collections;
7681
import java.util.List;
7782
import java.util.Map;
7883
import java.util.Set;
84+
import java.util.UUID;
7985
import java.util.stream.Collectors;
86+
import java.util.stream.Stream;
8087

8188
class DataServiceImpl implements DataService {
8289

@@ -88,6 +95,7 @@ class DataServiceImpl implements DataService {
8895
private final DocRefInfoService docRefInfoService;
8996
private final MetaService metaService;
9097
private final AttributeMapFactory attributeMapFactory;
98+
private final TempDirProvider tempDirProvider;
9199
private final SecurityContext securityContext;
92100
private final FeedStore feedStore;
93101

@@ -115,7 +123,8 @@ class DataServiceImpl implements DataService {
115123
final PipelineDataCache pipelineDataCache,
116124
final PipelineScopeRunnable pipelineScopeRunnable,
117125
final SourceConfig sourceConfig,
118-
final TaskContextFactory taskContextFactory) {
126+
final TaskContextFactory taskContextFactory,
127+
final TempDirProvider tempDirProvider) {
119128

120129
this.resourceStore = resourceStore;
121130
this.dataUploadTaskHandlerProvider = dataUploadTaskHandler;
@@ -125,6 +134,7 @@ class DataServiceImpl implements DataService {
125134
this.attributeMapFactory = attributeMapFactory;
126135
this.securityContext = securityContext;
127136
this.feedStore = feedStore;
137+
this.tempDirProvider = tempDirProvider;
128138

129139
this.dataFetcher = new DataFetcher(streamStore,
130140
feedProperties,
@@ -148,28 +158,53 @@ public ResourceGeneration download(final FindMetaCriteria criteria) {
148158
return securityContext.secureResult(AppPermission.EXPORT_DATA_PERMISSION, () -> {
149159
// Import file.
150160
final ResourceKey resourceKey = resourceStore.createTempFile("StroomData.zip");
151-
final Path file = resourceStore.getTempFile(resourceKey);
152-
String fileName = file.getFileName().toString();
153-
int index = fileName.lastIndexOf(".");
154-
if (index != -1) {
155-
fileName = fileName.substring(0, index);
156-
}
161+
final Path tempFile = resourceStore.getTempFile(resourceKey);
162+
final Path outputDir = tempDirProvider.get().resolve(UUID.randomUUID().toString());
163+
try {
164+
final DataDownloadSettings settings = new DataDownloadSettings();
165+
final DataDownloadResult result = dataDownloadTaskHandlerProvider.downloadData(
166+
criteria,
167+
outputDir,
168+
"data",
169+
settings);
170+
171+
if (result.getRecordsWritten() == 0) {
172+
if (result.getMessageList() != null && !result.getMessageList().isEmpty()) {
173+
throw new RuntimeException("Download failed with errors: " +
174+
result.getMessageList().stream()
175+
.map(Message::getMessage)
176+
.collect(Collectors.joining(", ")));
177+
}
178+
}
179+
180+
try {
181+
// Find out how many files were written.
182+
final List<Path> list;
183+
try (final Stream<Path> stream = Files.list(outputDir)) {
184+
list = stream.toList();
185+
}
157186

158-
final DataDownloadSettings settings = new DataDownloadSettings();
159-
final DataDownloadResult result = dataDownloadTaskHandlerProvider.downloadData(criteria,
160-
file.getParent(),
161-
fileName,
162-
settings);
163-
164-
if (result.getRecordsWritten() == 0) {
165-
if (result.getMessageList() != null && !result.getMessageList().isEmpty()) {
166-
throw new RuntimeException("Download failed with errors: " +
167-
result.getMessageList().stream()
168-
.map(Message::getMessage)
169-
.collect(Collectors.joining(", ")));
187+
if (list.size() == 1) {
188+
// If we have only 1 file then just move it to the temp file.
189+
Files.move(list.getFirst(), tempFile);
190+
} else if (list.size() > 1) {
191+
// If we have more than 1 file then zip them all.
192+
ZipUtil.zip(tempFile, outputDir);
193+
} else {
194+
throw new RuntimeException("Download failed with no files written");
195+
}
196+
} catch (final IOException e) {
197+
throw new RuntimeException("Download failed with errors: " + e.getMessage());
198+
}
199+
200+
return new ResourceGeneration(resourceKey, result.getMessageList());
201+
} finally {
202+
try {
203+
FileUtil.deleteDir(outputDir);
204+
} catch (final RuntimeException e) {
205+
LOGGER.error(e::getMessage, e);
170206
}
171207
}
172-
return new ResourceGeneration(resourceKey, result.getMessageList());
173208
});
174209
}
175210

@@ -191,11 +226,11 @@ public ResourceKey upload(final UploadDataRequest request) {
191226
return securityContext.secureResult(AppPermission.IMPORT_DATA_PERMISSION, () -> {
192227
try {
193228
// Import file.
194-
final Path file = resourceStore.getTempFile(request.getKey());
229+
final Path tempFile = resourceStore.getTempFile(request.getKey());
195230

196231
dataUploadTaskHandlerProvider.uploadData(
197232
request.getFileName(),
198-
file,
233+
tempFile,
199234
request.getFeedName(),
200235
request.getStreamTypeName(),
201236
request.getEffectiveMs(),
@@ -245,10 +280,10 @@ public List<DataInfoSection> info(final long id) {
245280
sortedKeys.forEach(key -> {
246281
final String value = attributeMap.get(key);
247282
if (value != null &&
248-
// We are going to add retention entries separately.
249-
!DataRetentionFields.RETENTION_AGE.equals(key) &&
250-
!DataRetentionFields.RETENTION_UNTIL.equals(key) &&
251-
!DataRetentionFields.RETENTION_RULE.equals(key)) {
283+
// We are going to add retention entries separately.
284+
!DataRetentionFields.RETENTION_AGE.equals(key) &&
285+
!DataRetentionFields.RETENTION_UNTIL.equals(key) &&
286+
!DataRetentionFields.RETENTION_RULE.equals(key)) {
252287

253288
if (MetaFields.DURATION.getFldName().equals(key)) {
254289
entries.add(new DataInfoSection.Entry(key, convertDuration(value)));
@@ -323,10 +358,7 @@ private String convertDuration(final String value) {
323358
private String convertTime(final String value) {
324359
try {
325360
long valLong = Long.parseLong(value);
326-
return DateUtil.createNormalDateTimeString(valLong)
327-
+ " ("
328-
+ valLong
329-
+ ")";
361+
return DateUtil.createNormalDateTimeString(valLong) + " (" + valLong + ")";
330362
} catch (RuntimeException e) {
331363
// Ignore.
332364
}
@@ -338,10 +370,7 @@ private String convertSize(final String value) {
338370
final long valLong = Long.parseLong(value);
339371
final String iecByteSizeStr = ModelStringUtil.formatIECByteSizeString(valLong);
340372
if (valLong >= 1024) {
341-
return iecByteSizeStr
342-
+ " ("
343-
+ NumberFormat.getIntegerInstance().format(valLong)
344-
+ ")";
373+
return iecByteSizeStr + " (" + NumberFormat.getIntegerInstance().format(valLong) + ")";
345374
} else {
346375
return iecByteSizeStr;
347376
}

Diff for: ‎stroom-data/stroom-data-store-impl/src/main/java/stroom/data/store/impl/DataUploadTaskHandler.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ private void uploadData(final TaskContext taskContext,
8484
final Long effectiveMs,
8585
final String metaData) {
8686
securityContext.secure(() -> {
87-
taskContext.info(file::toString);
87+
taskContext.info(() -> fileName);
8888
if (feedName == null) {
8989
throw new EntityServiceException("Feed not set!");
9090
}

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

+7-6
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package stroom.data.store.impl;
1818

1919
import stroom.resource.api.ResourceStore;
20-
import stroom.util.io.FileUtil;
2120
import stroom.util.io.StreamUtil;
2221
import stroom.util.shared.IsServlet;
2322
import stroom.util.shared.PropertyMap;
@@ -37,6 +36,7 @@
3736
import java.io.IOException;
3837
import java.io.InputStream;
3938
import java.io.OutputStream;
39+
import java.io.Serial;
4040
import java.nio.file.Files;
4141
import java.nio.file.Path;
4242
import java.util.HashMap;
@@ -50,9 +50,10 @@
5050
*/
5151
public final class ImportFileServlet extends HttpServlet implements IsServlet {
5252

53-
protected static final String FILE_UPLOAD_PROP_NAME = "fileUpload";
53+
private static final String FILE_UPLOAD_PROP_NAME = "fileUpload";
5454
private static final Logger LOGGER = LoggerFactory.getLogger(ImportFileServlet.class);
5555

56+
@Serial
5657
private static final long serialVersionUID = 487567988479000995L;
5758

5859
private static final Set<String> PATH_SPECS = Set.of("/importfile.rpc");
@@ -78,7 +79,7 @@ protected void doPost(final HttpServletRequest request, final HttpServletRespons
7879
try {
7980
// Parse the request and populate a map of file items.
8081
final Map<String, DiskFileItem> items = getFileItems(request);
81-
if (items.size() == 0) {
82+
if (items.isEmpty()) {
8283
response.getWriter().write(propertyMap.toArgLine());
8384
return;
8485
}
@@ -87,13 +88,13 @@ protected void doPost(final HttpServletRequest request, final HttpServletRespons
8788
Objects.requireNonNull(fileItem, "Property '" + FILE_UPLOAD_PROP_NAME + "' not found in request");
8889
final String fileName = fileItem.getName();
8990
final ResourceKey resourceKey = resourceStore.createTempFile(fileName);
90-
final Path file = resourceStore.getTempFile(resourceKey);
91+
final Path tempFile = resourceStore.getTempFile(resourceKey);
9192
streamEventLog.importStream(
9293
fileItem,
93-
FileUtil.getCanonicalPath(file),
94+
fileName,
9495
null);
9596
try (final InputStream inputStream = fileItem.getInputStream();
96-
final OutputStream outputStream = Files.newOutputStream(file)) {
97+
final OutputStream outputStream = Files.newOutputStream(tempFile)) {
9798
StreamUtil.streamToStream(inputStream, outputStream);
9899
}
99100

Diff for: ‎stroom-dictionary/stroom-dictionary-impl/src/main/java/stroom/dictionary/impl/DictionaryResourceImpl.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,9 @@ public ResourceGeneration download(final DocRef dictionaryRef) {
105105
}
106106

107107
final ResourceKey resourceKey = resourceStoreProvider.get().createTempFile("dictionary.txt");
108-
final Path file = resourceStoreProvider.get().getTempFile(resourceKey);
108+
final Path tempFile = resourceStoreProvider.get().getTempFile(resourceKey);
109109
try {
110-
Files.writeString(file, dictionary.getData(), StreamUtil.DEFAULT_CHARSET);
110+
Files.writeString(tempFile, dictionary.getData(), StreamUtil.DEFAULT_CHARSET);
111111
} catch (final IOException e) {
112112
LOGGER.error("Unable to download Dictionary", e);
113113
throw new UncheckedIOException(e);

0 commit comments

Comments
 (0)