[DSIP-45] Polish the Storage SPI by ruanwenjun · Pull Request #16141 · apache/dolphinscheduler

ruanwenjun · 2024-06-12T11:48:47Z

Purpose of the pull request

close #16140
close #16072

Brief change log

Refactor the storage api
Refactor the storage service
Add IT for LocalStorage/HdfsStorage/S3Storage
Remove some mock UT, this is meaningless.
Fix fetch file content and update file content will cut 3000 lines.

Verify this pull request

Verify by e2e(s3 case) / IT(local hdfs/ s3 case) and manual test.

Resource CRUD

UDF CRUD

Using in task

...torage-s3/src/main/java/org/apache/dolphinscheduler/plugin/storage/s3/S3StorageOperator.java

...ler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java

+            response.setContentType("application/octet-stream");
+            response.setCharacterEncoding("utf-8");
+            response.setContentLength(length);
+            response.setHeader("Content-Disposition", "attachment;filename=" + fileName);


...ler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java

+            response.setCharacterEncoding("utf-8");
+            response.setContentLength(length);
+            response.setHeader("Content-Disposition", "attachment;filename=" + fileName);
+            Files.copy(Paths.get(localTmpFileAbsolutePath), response.getOutputStream());


...eduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ResourcesController.java

...ler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java

+        String originDirectoryAbsolutePath = renameDirectoryDto.getOriginDirectoryAbsolutePath();
+        String targetDirectoryAbsolutePath = renameDirectoryDto.getTargetDirectoryAbsolutePath();
+        storageOperate.copy(originDirectoryAbsolutePath, targetDirectoryAbsolutePath, true, true);
+        log.info("Success rename directory: {} -> {} ", originDirectoryAbsolutePath, targetDirectoryAbsolutePath);


...ler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java

+        String originFileAbsolutePath = renameFileDto.getOriginFileAbsolutePath();
+        String targetFileAbsolutePath = renameFileDto.getTargetFileAbsolutePath();
+        storageOperate.copy(originFileAbsolutePath, targetFileAbsolutePath, true, true);
+        log.info("Success rename file: {} -> {} ", originFileAbsolutePath, targetFileAbsolutePath);


...ler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java

+        String originFileAbsolutePath = renameFileDto.getOriginFileAbsolutePath();
+        String targetFileAbsolutePath = renameFileDto.getTargetFileAbsolutePath();
+        storageOperate.copy(originFileAbsolutePath, targetFileAbsolutePath, true, true);
+        log.info("Success rename file: {} -> {} ", originFileAbsolutePath, targetFileAbsolutePath);


...ler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java

-            }
+            storageOperate.upload(srcLocalTmpFileAbsolutePath, updateFileDto.getFileAbsolutePath(), true, true);
+            ApiServerMetrics.recordApiResourceUploadSize(updateFileDto.getFile().getSize());
+            log.info("Success upload resource file: {} complete.", updateFileDto.getFileAbsolutePath());


...ler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java

+            storageOperate.upload(srcLocalTmpFileAbsolutePath, updateFileFromContentDto.getFileAbsolutePath(), true,
+                    true);
+            ApiServerMetrics.recordApiResourceUploadSize(updateFileFromContentDto.getFileContent().length());
+            log.info("Success upload resource file: {} complete.", updateFileFromContentDto.getFileAbsolutePath());


...torage-api/src/main/java/org/apache/dolphinscheduler/plugin/storage/api/StorageOperator.java

codecov-commenter · 2024-06-12T15:02:05Z

Codecov Report

Attention: Patch coverage is 41.84477% with 517 lines in your changes missing coverage. Please review.

Project coverage is 41.07%. Comparing base (61a6bd0) to head (12b3efd).

❗ Current head 12b3efd differs from pull request most recent head 82a6f2d

Please upload reports for the commit 82a6f2d to get more accurate results.

Files	Patch %	Lines
...heduler/api/service/impl/ResourcesServiceImpl.java	0.00%	147 Missing ⚠️
...nscheduler/api/controller/ResourcesController.java	50.57%	43 Missing ⚠️
...scheduler/plugin/storage/s3/S3StorageOperator.java	77.61%	25 Missing and 5 partials ⚠️
...pache/dolphinscheduler/common/utils/FileUtils.java	7.40%	25 Missing ⚠️
...or/resource/CreateDirectoryRequestTransformer.java	4.16%	23 Missing ⚠️
...resource/PagingResourceItemRequestTransformer.java	0.00%	23 Missing ⚠️
...duler/plugin/storage/hdfs/HdfsStorageOperator.java	79.79%	18 Missing and 2 partials ⚠️
...or/resource/FileFromContentRequestTransformer.java	9.52%	19 Missing ⚠️
...api/validator/resource/FileRequestTransformer.java	9.52%	19 Missing ⚠️
...apache/dolphinscheduler/api/vo/ResourceItemVO.java	0.00%	14 Missing ⚠️
... and 28 more

Additional details and impacted files

@@             Coverage Diff              @@
##                dev   #16141      +/-   ##
============================================
+ Coverage     40.70%   41.07%   +0.36%     
+ Complexity     5245     5155      -90     
============================================
  Files          1385     1399      +14     
  Lines         46109    44856    -1253     
  Branches       4923     4756     -167     
============================================
- Hits          18768    18423     -345     
+ Misses        25414    24628     -786     
+ Partials       1927     1805     -122

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

rickchengx

Overall LGTM, thanks @ruanwenjun for refactoring Storage API to enhance the robustness and maintainability of this part of the code.

...ler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java

rickchengx

Since this is a refactor of the storage module, it would be better to provide some screenshots.

ruanwenjun · 2024-06-15T03:34:38Z

Since this is a refactor of the storage module, it would be better to provide some screenshots.

Good suggestion, I add some picture in the description

SbloodyS

Please check failed UT.

ruanwenjun · 2024-06-16T13:27:20Z

Please check failed UT.

Done

SbloodyS

LGTM.

rickchengx

LGTM

sonarqubecloud · 2024-06-17T06:05:24Z

Quality Gate failed

Failed conditions
32.2% Coverage on New Code (required ≥ 60%)
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

ruanwenjun requested review from Amy0104, EricGao888, SbloodyS, caishunfeng and songjianet as code owners June 12, 2024 11:48

github-actions bot added UI ui and front end related backend labels Jun 12, 2024

ruanwenjun marked this pull request as draft June 12, 2024 11:58

ruanwenjun force-pushed the dev_wenjun_refactorStorageAPI branch 2 times, most recently from d1333cf to a0db19d Compare June 12, 2024 12:09

github-advanced-security bot found potential problems Jun 12, 2024

View reviewed changes

ruanwenjun force-pushed the dev_wenjun_refactorStorageAPI branch 3 times, most recently from 448904a to 02350b4 Compare June 12, 2024 14:21

ruanwenjun self-assigned this Jun 12, 2024

ruanwenjun added DSIP 3.3.0 labels Jun 12, 2024

ruanwenjun added this to the 3.3.0 milestone Jun 12, 2024

ruanwenjun force-pushed the dev_wenjun_refactorStorageAPI branch 3 times, most recently from 1cff355 to b89b35b Compare June 12, 2024 14:32

github-advanced-security bot found potential problems Jun 12, 2024

View reviewed changes

...torage-api/src/main/java/org/apache/dolphinscheduler/plugin/storage/api/StorageOperator.java Fixed Show fixed Hide fixed

ruanwenjun force-pushed the dev_wenjun_refactorStorageAPI branch from b89b35b to 554b9f2 Compare June 13, 2024 01:19

ruanwenjun marked this pull request as ready for review June 13, 2024 12:09

ruanwenjun force-pushed the dev_wenjun_refactorStorageAPI branch 3 times, most recently from 8c6a08c to 41332d7 Compare June 14, 2024 15:34

ruanwenjun mentioned this pull request Jun 14, 2024

Fix HdfsStorageOperator will not load the hdfs-site configured at common.properties #16154

Closed

ruanwenjun added the priority:middle label Jun 14, 2024

ruanwenjun force-pushed the dev_wenjun_refactorStorageAPI branch 2 times, most recently from 346663b to c3253e9 Compare June 15, 2024 02:32

rickchengx reviewed Jun 15, 2024

View reviewed changes

...ler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java Show resolved Hide resolved

...ler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java Show resolved Hide resolved

rickchengx reviewed Jun 15, 2024

View reviewed changes

Refactory Storage API

80d9adb

ruanwenjun force-pushed the dev_wenjun_refactorStorageAPI branch from c3253e9 to 80d9adb Compare June 15, 2024 03:29

ruanwenjun requested a review from rickchengx June 15, 2024 03:38

Add HDFS ut case

2fa31dc

ruanwenjun force-pushed the dev_wenjun_refactorStorageAPI branch from 2bf30e1 to 2fa31dc Compare June 16, 2024 03:28

SbloodyS reviewed Jun 16, 2024

View reviewed changes

ruanwenjun force-pushed the dev_wenjun_refactorStorageAPI branch from 72ab21f to 8876467 Compare June 16, 2024 07:12

ruanwenjun force-pushed the dev_wenjun_refactorStorageAPI branch from 8876467 to 6964ceb Compare June 16, 2024 13:55

Fix ut

9fb32f3

ruanwenjun force-pushed the dev_wenjun_refactorStorageAPI branch from 6964ceb to 9fb32f3 Compare June 16, 2024 13:58

ruanwenjun requested a review from SbloodyS June 17, 2024 02:06

SbloodyS approved these changes Jun 17, 2024

View reviewed changes

rickchengx approved these changes Jun 17, 2024

View reviewed changes

Merge branch 'dev' into dev_wenjun_refactorStorageAPI

82a6f2d

ruanwenjun merged commit edeb1b2 into apache:dev Jun 17, 2024

ruanwenjun deleted the dev_wenjun_refactorStorageAPI branch June 17, 2024 07:25

Conversation

ruanwenjun commented Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of the pull request

Brief change log

Verify this pull request

Resource CRUD

UDF CRUD

Using in task

Uh oh!

Uh oh!

Check warning

Check failure

Uh oh!

Uh oh!

Check failure

Check failure

Check failure

Check failure

Check failure

Uh oh!

codecov-commenter commented Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rickchengx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rickchengx left a comment

Choose a reason for hiding this comment

Uh oh!

ruanwenjun commented Jun 15, 2024

Uh oh!

SbloodyS left a comment

Choose a reason for hiding this comment

Uh oh!

ruanwenjun commented Jun 16, 2024

Uh oh!

SbloodyS left a comment

Choose a reason for hiding this comment

Uh oh!

rickchengx left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jun 17, 2024

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ruanwenjun commented Jun 12, 2024 •

edited

Loading

codecov-commenter commented Jun 12, 2024 •

edited

Loading