Persist dump file content for dynamic simulation #116

thangqp · 2024-10-28T19:42:25Z

Dump file exported by dynamic simulation is persisted in db in order to reuse as one of inputs for dynamic security analysis

Signed-off-by: Thang PHAM <[email protected]>

ghazwarhili · 2024-11-13T14:01:01Z

src/main/java/org/gridsuite/ds/server/utils/Utils.java

+        try (InputStream is = Files.newInputStream(filePath);
+             ByteArrayOutputStream os = new ByteArrayOutputStream()) {
+            // Important: zipOs must be outside of try to call manually close() in order to add EOF to os
+            ZipOutputStream zipOs = new ZipOutputStream(os);


you can use the zipOs whithin the try with resource block to avoid the call of close() manually ? as it will be closed automatically by the try block finishes

DONE in 85a0baa

ghazwarhili · 2024-11-13T14:07:37Z

src/main/java/org/gridsuite/ds/server/utils/Utils.java

+                zipOs.write(buffer, 0, length);
+            }
+            // call close() manually to add EOF to os
+            zipOs.close();


you can call the closeEntry() to finalize the entry which is more explicit

DONE in 85a0baa

ghazwarhili · 2024-11-13T14:15:46Z

src/main/java/org/gridsuite/ds/server/utils/Utils.java

+            zipOs.close();
+            return os.toByteArray();
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);


add message to improve debugging if an error occurs

Suggested change

throw new UncheckedIOException(e);

throw new UncheckedIOException("Failed to zip the provided file", e);

DONE in 85a0baa

ghazwarhili · 2024-11-13T14:17:12Z

src/main/java/org/gridsuite/ds/server/utils/Utils.java

+                }
+            }
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);


the same here

Suggested change

throw new UncheckedIOException(e);

throw new UncheckedIOException("Failed to unzip the provided byte array", e);

DONE in 85a0baa

ghazwarhili · 2024-11-13T14:19:45Z

src/main/java/org/gridsuite/ds/server/utils/Utils.java

+                while ((length = zipIs.read(buffer)) > 0) {
+                    fos.write(buffer, 0, length);
+                }
+            }


you need to close properly the entry
zipIs.closeEntry()

DONE in 85a0baa

ghazwarhili · 2024-11-13T14:54:07Z

src/main/java/org/gridsuite/ds/server/service/DynamicSimulationWorkerService.java

+            runContext.setWorkDir(workDir);
+
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);


Suggested change

throw new UncheckedIOException(e);

throw new UncheckedIOException("Error creating temporary working directory for dynamo simulation", e);

DONE in 85a0baa

ghazwarhili · 2024-11-13T14:56:51Z

src/main/java/org/gridsuite/ds/server/service/DynamicSimulationWorkerService.java

+        Path dumpDir = workDir.resolve("dump");
+        FileUtil.createDirectory(dumpDir);
+        DynawoSimulationParameters dynawoSimulationParameters = parameters.getExtension(DynawoSimulationParameters.class);
+        dynawoSimulationParameters.setDumpFileParameters(DumpFileParameters.createExportDumpFileParameters(dumpDir));


parameters.getExtension(...) could return null ?
you can use optional.ofNullable(...) to make it safer otherwise throw exception

The extension is created in the previous step https://github.com/gridsuite/dynamic-simulation-server/blob/main/src/main/java/org/gridsuite/ds/server/service/parameters/impl/ParametersServiceImpl.java#L109, so it is never null.

Otherwise, throw another exception is not better than NPE, anyway the program must be stopped, the difference is instead of returning to end user a technical exception, we return a functional exception.

ghazwarhili · 2024-11-13T15:14:14Z

src/main/java/org/gridsuite/ds/server/service/DynamicSimulationWorkerService.java

+            }
+        }
+
+        updateResult(resultContext.getResultUuid(), result, outputState);


move and extract this logic from saveResult into small helper methods to reduce the complexity of the new code

Path dumpDir = getDumpFilePath(resultContext);

byte[] outputState = null; and if (dumpDir != null) { outputState = zipDumpFile(dumpDir); }

updateResult(resultContext.getResultUuid(), result, outputState);

DONE in 85a0baa

ghazwarhili · 2024-11-13T15:24:25Z

src/test/java/org/gridsuite/ds/server/controller/DynamicSimulationControllerIEEE14Test.java

+
+        // check dump file not empty
+        byte[] outputState = dynamicSimulationResultService.getOutputState(runUuid);
+        assertThat(outputState).isNotEmpty();


Suggested change

assertThat(outputState).isNotEmpty();

assertThat(outputState)

.withFailMessage("Output state for the dynamo simulation is empty ...")

.isNotEmpty();

DONE in 85a0baa

ghazwarhili · 2024-11-13T15:29:36Z

src/test/java/org/gridsuite/ds/server/controller/DynamicSimulationControllerIEEE14Test.java

+        // check dump file not empty
+        byte[] outputState = dynamicSimulationResultService.getOutputState(runUuid);
+        assertThat(outputState).isNotEmpty();
+        logger.info("Size of zipped output state = {} KB ", outputState.length / 1024);


are you sure about the logging level ?
shouldn't be logging in debug level here ?

this is test code, I prefer to show important info in any mode to facilitate analysis

Signed-off-by: Thang PHAM <[email protected]>

ghazwarhili

Code => OK
Test => OK

etiennehomer · 2024-11-15T14:36:41Z

src/main/java/org/gridsuite/ds/server/utils/Utils.java

@@ -84,4 +90,38 @@ public static Property convertProperty(BasicProperty property) {

        return propertyBuilder.build();
    }
+
+    public static byte[] zip(Path filePath) {


Gzip would be more appropriated for the compression of one file

DONE in f295c86

etiennehomer · 2024-11-15T14:44:36Z

src/main/java/org/gridsuite/ds/server/model/ResultEntity.java

@@ -46,4 +47,7 @@ public ResultEntity(UUID id, UUID timeSeriesId, UUID timeLineId, DynamicSimulati
    @Enumerated(EnumType.STRING)
    private DynamicSimulationStatus status;

+    @Column(name = "outputState")
+    private byte[] outputState;


It's a problem to have the file content in the same entity and same table. See dynamicSimulationService.findStatus(). You request the whole file just to get the computation status

A solution is using projection for select and and jpql for update lay avoid loading outputState in memory.
DONE in 87f095b

Signed-off-by: Thang PHAM <[email protected]>

sonarqubecloud · 2024-11-18T12:04:38Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
88.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

thangqp added 4 commits October 28, 2024 20:40

Export dump file for dynamic simulation

8a3d9ea

create root storage dir

20bfc35

create dumps folder inside computationManager local dir

46f2b4b

zip dump content then persist into db

cce9e00

thangqp changed the title ~~Export dump file for dynamic simulation~~ Persist dump file content for dynamic simulation Oct 30, 2024

use new version powsybl-ws-commons 1.16.0-SNAPSHOT

606308d

Signed-off-by: Thang PHAM <[email protected]>

thangqp requested review from etiennehomer and jonenst October 30, 2024 11:42

thangqp added 7 commits October 30, 2024 13:51

do not group dump folder inside a shared dumps folder

6fae1e2

Signed-off-by: Thang PHAM <[email protected]>

working directory

8380dc5

Signed-off-by: Thang PHAM <[email protected]>

show size of zipped dump file in test

70d56a6

Signed-off-by: Thang PHAM <[email protected]>

Add comment for close manually ZipOutputStream

621227e

Signed-off-by: Thang PHAM <[email protected]>

Remove columnDefinition for outputState

965a38e

Signed-off-by: Thang PHAM <[email protected]>

Merge from main

a424c8c

Signed-off-by: Thang PHAM <[email protected]>

revert junit vintage

d2f45d1

Signed-off-by: Thang PHAM <[email protected]>

thangqp requested a review from ghazwarhili November 13, 2024 09:57

ghazwarhili reviewed Nov 13, 2024

View reviewed changes

Rectify code following Ghazwa comment

85a0baa

Signed-off-by: Thang PHAM <[email protected]>

ghazwarhili approved these changes Nov 14, 2024

View reviewed changes

etiennehomer reviewed Nov 15, 2024

View reviewed changes

thangqp added 2 commits November 15, 2024 16:32

ZIP to GZIP

f295c86

Signed-off-by: Thang PHAM <[email protected]>

using projection and update result without finding by jpa

87f095b

Signed-off-by: Thang PHAM <[email protected]>

thangqp requested review from etiennehomer and ghazwarhili November 18, 2024 12:09

etiennehomer approved these changes Nov 18, 2024

View reviewed changes

thangqp merged commit 38af3d8 into main Nov 18, 2024
3 checks passed

thangqp deleted the ds_export_dump_file branch November 18, 2024 13:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Persist dump file content for dynamic simulation #116

Persist dump file content for dynamic simulation #116

thangqp commented Oct 28, 2024 •

edited

Loading

ghazwarhili Nov 13, 2024

thangqp Nov 13, 2024

thangqp Nov 13, 2024

ghazwarhili Nov 13, 2024

thangqp Nov 13, 2024

thangqp Nov 13, 2024

ghazwarhili Nov 13, 2024

thangqp Nov 13, 2024

ghazwarhili Nov 13, 2024

thangqp Nov 13, 2024

ghazwarhili Nov 13, 2024

thangqp Nov 13, 2024

thangqp Nov 13, 2024

ghazwarhili Nov 13, 2024

thangqp Nov 13, 2024

ghazwarhili Nov 13, 2024

thangqp Nov 13, 2024

ghazwarhili Nov 13, 2024

thangqp Nov 13, 2024

ghazwarhili Nov 13, 2024

thangqp Nov 13, 2024

ghazwarhili Nov 13, 2024

thangqp Nov 13, 2024

thangqp Nov 13, 2024

ghazwarhili left a comment

etiennehomer Nov 15, 2024

thangqp Nov 15, 2024 •

edited

Loading

etiennehomer Nov 15, 2024

thangqp Nov 15, 2024 •

edited

Loading

sonarqubecloud bot commented Nov 18, 2024

	throw new UncheckedIOException(e);
	throw new UncheckedIOException("Failed to zip the provided file", e);

	throw new UncheckedIOException(e);
	throw new UncheckedIOException("Error creating temporary working directory for dynamo simulation", e);

Persist dump file content for dynamic simulation #116

Persist dump file content for dynamic simulation #116

Conversation

thangqp commented Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghazwarhili left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thangqp Nov 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thangqp Nov 15, 2024 • edited Loading

Choose a reason for hiding this comment

sonarqubecloud bot commented Nov 18, 2024

Quality Gate passed

thangqp commented Oct 28, 2024 •

edited

Loading

thangqp Nov 15, 2024 •

edited

Loading

thangqp Nov 15, 2024 •

edited

Loading