Fix errorprone and Javadoc warnings #2614

inv-jishnu · 2025-04-30T04:25:19Z

Description

This PR fixes error prone and java doc warnings and renamed logger param used from LOGGER to logger.
This PR was created based on following feedback from PR #2591.

Related issues and/or PRs

Add import log classes and utils #2591

Changes made

I have made the following changes

Renamed LOGGER to logger
Changed assert to assertThat
Added SAME_NAME_BUT_DIFFERENT to ignore patterns that need to be ignored for data loader core project
Added suppress warning for SameNameButDifferent and MissingSummary and a few more.
Fixed few warning with minor code changes
Removed an unused class and a method

Checklist

The following is a best-effort checklist. If any items in this checklist are not applicable to this PR or are dependent on other, unmerged PRs, please still mark the checkboxes after you have read and understood each item.

I have commented my code, particularly in hard-to-understand areas.
I have updated the documentation to reflect the changes.
I have considered whether similar issues could occur in other products, components, or modules if this PR is for bug fixes.
Any remaining open issues linked to this PR are documented and up-to-date (Jira, GitHub, etc.).
Tests (unit, integration, etc.) have been added for the changes.
My changes generate no new warnings.
Any dependent changes in other PRs have been merged and published.

Additional notes (optional)

NA

Release notes

Fix warnings for java doc and error prone

…rnings

inv-jishnu · 2025-06-11T14:04:30Z

@brfrn169 san, @komamitsu san, @Torch3333 san,

I have made a change to use mutate instead of put in 0f03536 as there was a deprecation warning.

Also there are error: cannot find symbol warning remaining at 4 places which is related to lombok (lombok version and java version)
ex.
scalardb/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/ExportOptions.java:43: error: cannot find symbol
public static ExportOptionsBuilder builder(
^
symbol: class ExportOptionsBuilder

inv-jishnu · 2025-06-11T14:26:27Z

Removed a string format constant to remove a warning in bfad6d3.

scalardb/data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommand.java:48: warning: [InlineFormatString] Prefer to create format strings inline, instead of extracting them to a single-use constant
  private static final String EXPORT_FILE_NAME_FORMAT = "export.%s.%s.%s.%s";
                              ^
    (see https://errorprone.info/bugpattern/InlineFormatString)

brfrn169 · 2025-06-12T00:28:48Z

@inv-jishnu I found one more warning:

/Users/toshihiro.suzuki/work/scalar-labs/scalardb/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/processor/ImportProcessor.java:90: warning: [FutureReturnValueIgnored] Return value of methods returning Future must be checked. Ignoring returned Futures suppresses exceptions thrown from the code that completes the Future.
          dataChunkProcessorExecutor.submit(
                                           ^
    (see https://errorprone.info/bugpattern/FutureReturnValueIgnored)
  Did you mean to remove this line?

brfrn169 · 2025-06-12T00:42:30Z

data-loader/core/src/main/java/com/scalar/db/dataloader/core/ScanRange.java

@@ -4,6 +4,7 @@
 import lombok.Value;

 /** * The scan range which is used in data export scan filtering */


Suggested change

/** * The scan range which is used in data export scan filtering */

/** The scan range which is used in data export scan filtering. */

brfrn169 · 2025-06-12T00:42:51Z

...r/core/src/main/java/com/scalar/db/dataloader/core/dataimport/datachunk/ImportDataChunk.java

@@ -4,7 +4,8 @@
 import lombok.Builder;
 import lombok.Data;

-/** * Import data chunk data */
+/** * Import data chunk data. */


Suggested change

/** * Import data chunk data. */

/** Import data chunk data. */

brfrn169 · 2025-06-12T00:43:00Z

.../src/main/java/com/scalar/db/dataloader/core/dataimport/datachunk/ImportDataChunkStatus.java

@@ -6,7 +6,8 @@
 import lombok.Builder;
 import lombok.Data;

-/** * A DTO to store import data chunk details */
+/** * A DTO to store import data chunk details. */


Suggested change

/** * A DTO to store import data chunk details. */

/** A DTO to store import data chunk details. */

brfrn169 · 2025-06-12T00:43:26Z

...n/java/com/scalar/db/dataloader/core/dataimport/transactionbatch/ImportTransactionBatch.java

@@ -6,6 +6,7 @@
 import lombok.Value;

 /** Transaction batch details */


Suggested change

/** Transaction batch details */

/** Transaction batch details. */

brfrn169 · 2025-06-12T00:43:34Z

.../com/scalar/db/dataloader/core/dataimport/transactionbatch/ImportTransactionBatchResult.java

@@ -8,6 +8,7 @@
 import lombok.Value;

 /** Transaction batch result */


Suggested change

/** Transaction batch result */

/** Transaction batch result. */

brfrn169 · 2025-06-12T00:44:49Z

gradle/spotbugs-exclude.xml

@@ -37,7 +37,7 @@
  </Match>
  <!-- Ignore mutable object exposure warnings(caused by Lombok) for all classes in dataloader.core -->
  <Match>
-    <Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2,CT_CONSTRUCTOR_THROW"/>
+    <Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2,CT_CONSTRUCTOR_THROW,SAME_NAME_BUT_DIFFERENT"/>


CT_CONSTRUCTOR_THROW and SAME_NAME_BUT_DIFFERENT are already excluded globally:

Suggested change

<Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2,CT_CONSTRUCTOR_THROW,SAME_NAME_BUT_DIFFERENT"/>

<Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2"/>

brfrn169 · 2025-06-12T00:52:08Z

data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/dao/ScalarDbDao.java

@@ -107,7 +108,7 @@ public void put(

    Put put = createPutWith(namespace, table, partitionKey, clusteringKey, columns);
    try {
-      transaction.put(put);
+      transaction.mutate(Collections.singletonList(put));


I don’t think we should use mutate() for a single mutation. Please continue using put() for now.

@brfrn169 san,
I will revert this change.
Thank you.

brfrn169 · 2025-06-12T01:02:00Z

@inv-jishnu I also found many Javadoc errors and warnings. Please handle them in a separate PR.

Torch3333

LGTM, thank you!

Torch3333

LGTM, thank you!

This reverts commit 0f03536.

inv-jishnu · 2025-06-12T04:21:09Z

@brfrn169 san,

Apart from error: cannot find symbol and a few deprecated warnings for put and Scan.Ordering (in CLI). I have fixed all warning that I have seen. If I missed any I will update the rest in another PR.
Thank you.

brfrn169 · 2025-06-13T00:48:27Z

Apart from error: cannot find symbol and a few deprecated warnings for put and Scan.Ordering (in CLI). I have fixed all warning that I have seen.

@inv-jishnu Did you also mean Javadoc warnings? I still see many warnings like warning: no @param for ..., warning: no @return, and warning: no @throws ..., other than error: cannot find symbol.

Also, regarding Scan.Ordering, you can use the Scan.Ordering.asc() or Scan.Ordering.desc() methods to remove the warning.

feeblefakie

LGTM! Thank you!

ypeckstadt

LGTM. Thank you.

On my side I don't see any errors or warnings anymore when I build it with Java 8. I do get a lot more warnings when building with Java 17 selected though but probably can be ignored for now.

brfrn169

LGTM! Thank you!

inv-jishnu and others added 21 commits April 10, 2025 15:10

Initial commit

be9ee23

Spotless applied again

89b9f05

Removed unused code

49c83b6

Merge branch 'master' into feat/data-loader/import-log-2

b2871fb

Removed unused classes and references

c5c9c0a

Merge branch 'master' into feat/data-loader/import-log-2

4964e8d

Merge branch 'master' into feat/data-loader/import-log-2

ff81f5f

Improve Javadocs

3934c2a

Changes

9958f95

Renamed parameters

1afbc21

logging changes

8c5114d

removed repeated code

ffab395

Merge branch 'master' into feat/data-loader/import-log-2

79df1ed

Merge branch 'master' into feat/data-loader/import-log-2

cf31672

Added excetpion throw

6dd213e

Synchronisation changes

6542177

Added volatile back to fix spotbugs issue

603e46e

Removed unused variable

eaf9d88

Chanaged LOGGER to logger

502034e

logger name change in test

6b22d1a

Fix warnings and rename logger

7d5ad8c

inv-jishnu added the enhancement New feature or request label Apr 30, 2025

inv-jishnu self-assigned this Apr 30, 2025

inv-jishnu marked this pull request as draft April 30, 2025 04:25

inv-jishnu mentioned this pull request Apr 30, 2025

Add import log classes and utils #2591

Merged

6 tasks

Fix added for warning and supress warning added for lombok related wa…

afd58f4

…rnings

Base automatically changed from feat/data-loader/import-log-2 to master May 8, 2025 04:09

inv-jishnu and others added 3 commits May 14, 2025 09:31

Merged latest changes from master after resolving conflicts

3652f10

Merge branch 'master' into fix/data-loader/logger-rename

cc5365b

Merge branch 'master' into fix/data-loader/logger-rename

570ab38

inv-jishnu and others added 3 commits June 11, 2025 18:45

Added annotation for same name but different

3de9ad7

Merge branch 'master' into fix/data-loader/logger-rename

f5bb662

Changd put to mutate

0f03536

Removed constant formatter to fix warning

bfad6d3

brfrn169 reviewed Jun 12, 2025

View reviewed changes

brfrn169 requested a review from ypeckstadt June 12, 2025 00:53

Torch3333 approved these changes Jun 12, 2025

View reviewed changes

inv-jishnu added 2 commits June 12, 2025 09:08

Revert "Changd put to mutate"

be503d6

This reverts commit 0f03536.

Changes

ac03202

inv-jishnu requested a review from brfrn169 June 12, 2025 04:22

Merge branch 'master' into fix/data-loader/logger-rename

862bbb2

feeblefakie reviewed Jun 13, 2025

View reviewed changes

inv-jishnu and others added 8 commits June 13, 2025 11:18

Scan order updated

eecf28a

Merge branch 'master' into fix/data-loader/logger-rename

7e3784b

Fixed symbol not found issue

e179656

Javadoc related fixes

f75d31b

Java doc related changes -2

06a4a9a

Javadoc changes and removed an unused file

db73e47

Merge branch 'master' into fix/data-loader/logger-rename

0d30850

Merge branch 'master' into fix/data-loader/logger-rename

a2756c1

ypeckstadt approved these changes Jun 17, 2025

View reviewed changes

brfrn169 approved these changes Jun 17, 2025

View reviewed changes

brfrn169 changed the title ~~Fix errorprone warnings~~ Fix errorprone and Javadoc warnings Jun 17, 2025

		@@ -4,6 +4,7 @@
		import lombok.Value;

		/** * The scan range which is used in data export scan filtering */

	/** * The scan range which is used in data export scan filtering */
	/** The scan range which is used in data export scan filtering. */

	/** * Import data chunk data. */
	/** Import data chunk data. */

	/** * A DTO to store import data chunk details. */
	/** A DTO to store import data chunk details. */

		@@ -6,6 +6,7 @@
		import lombok.Value;

		/** Transaction batch details */

	/** Transaction batch details */
	/** Transaction batch details. */

		@@ -8,6 +8,7 @@
		import lombok.Value;

		/** Transaction batch result */

	/** Transaction batch result */
	/** Transaction batch result. */

	<Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2,CT_CONSTRUCTOR_THROW,SAME_NAME_BUT_DIFFERENT"/>
	<Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2"/>

Fix errorprone and Javadoc warnings #2614

Are you sure you want to change the base?

Fix errorprone and Javadoc warnings #2614

Conversation

inv-jishnu commented Apr 30, 2025 • edited by ypeckstadt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues and/or PRs

Changes made

Checklist

Additional notes (optional)

Release notes

Uh oh!

inv-jishnu commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

inv-jishnu commented Jun 11, 2025

Uh oh!

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

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brfrn169 commented Jun 12, 2025

Uh oh!

Torch3333 left a comment

Choose a reason for hiding this comment

Uh oh!

Torch3333 left a comment

Choose a reason for hiding this comment

Uh oh!

inv-jishnu commented Jun 12, 2025

Uh oh!

brfrn169 commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feeblefakie left a comment

Choose a reason for hiding this comment

Uh oh!

ypeckstadt left a comment

Choose a reason for hiding this comment

Uh oh!

brfrn169 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

inv-jishnu commented Apr 30, 2025 •

edited by ypeckstadt

Loading

inv-jishnu commented Jun 11, 2025 •

edited

Loading

brfrn169 commented Jun 12, 2025 •

edited

Loading

brfrn169 commented Jun 13, 2025 •

edited

Loading