refactor: use commons module in server

[ata-nas]

Description:

• refactor :server to start using logic from :common

Related issue(s):
Fixes #252

Notes for reviewer:

• in the places where com.hedera.block.common.utils.FileUtilities#createPathIfNotExists is used, the previous alternative used Files#createDirectory, where the new version uses Files#createDirectories which acts quite differently, in fact, one unit test, namely testProvidesBlockWriter_IOException, which expects an exception to be thrown was not able to run successfully since it now creates the path using the new implementation with Files#createDirectories and previously it was not able to (the test is now modified). We should be careful about this since it is possible that the new usage could lead to silent bugs if unit tests do not already cover the needed edge cases.
• have I missed any places to refactor?

Checklist

• refactor com.hedera.block.server.Constants remove app.properties and logging.properties and use them from com.hedera.block.common.constants.StringsConstants;
• remove com.hedera.block.server.persistence.storage.FileUtils and start using com.hedera.block.common.utils.FileUtilities instead;

---

• Documented (Code comments, README, etc.)
• Tested (unit, integration, etc.) - N/A

[AlfredoG87]

LGTM 👍

[jsync-swirlds]

Still reviewing, but these changes are not correct.

[jsync-swirlds]

That's the rest reviewed.
Let's chat about this code, as there are obviously some gaps in terms of understanding why some things are written the way they were.

[ata-nas]

@jsync-swirlds thanks for the reviews.

Keep in mind that when I was doing these changes (as well as the ones in the simulator in the other PR), my aim was to only kind of 'redirect' all current usages of the utilities and constants to the new common module that we introduced. That is why I have left everything as is and have not made any significant changes, but took the approach to modify a bit the common module in order to fit the 'redirection', which I do agree with you is not a good thing to do.

I have fixed the obvious and commented on the rest, but I agree, lets have a conversation in general about the topics here and others as well, since this would help me to gain more insight into the Hedera philosophy and the team's specific views on what we should aim for in terms of implementation and thought process. This will help avoid such big gaps and so many questions in the future. Thanks! :)

[jsync-swirlds]

@jsync-swirlds thanks for the reviews.

Keep in mind that when I was doing these changes (as well as the ones in the simulator in the other PR), my aim was to only kind of 'redirect' all current usages of the utilities and constants to the new common module that we introduced. That is why I have left everything as is and have not made any significant changes, but took the approach to modify a bit the common module in order to fit the 'redirection', which I do agree with you is not a good thing to do.

I have fixed the obvious and commented on the rest, but I agree, lets have a conversation in general about the topics here and others as well, since this would help me to gain more insight into the Hedera philosophy and the team's specific views on what we should aim for in terms of implementation and thought process. This will help avoid such big gaps and so many questions in the future. Thanks! :)

I understand the intent. I think (as we discussed) that you would be better off (and the result will be better designed) to keep the shared library as clean as practical, and make whatever changes are needed in the server to correctly use that library.

[jsync-swirlds]

Just one nitpick, which could be addressed later.

[ata-nas]

address this comment as well

[jsync-swirlds]

There are a couple items noted that should be addressed as a separate issue.
Please file issues for those prior to merging this PR.

[mattp-swirldslabs]

Looks good

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.88%. Comparing base (b6c93e6) to head (1cc7962).
Report is 4 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #272      +/-   ##
============================================
- Coverage     99.35%   98.88%   -0.48%     
+ Complexity      295      288       -7     
============================================
  Files            60       59       -1     
  Lines          1092     1077      -15     
  Branches         78       76       -2     
============================================
- Hits           1085     1065      -20     
- Misses            5        8       +3     
- Partials          2        4       +2     

Files with missing lines	Coverage Δ
...a/com/hedera/block/common/utils/FileUtilities.java	100.00% <100.00%> (ø)
.../persistence/storage/PersistenceStorageConfig.java	100.00% <100.00%> (ø)
...ver/persistence/storage/read/BlockAsDirReader.java	100.00% <100.00%> (ø)
...sistence/storage/read/BlockAsDirReaderBuilder.java	100.00% <100.00%> (ø)
.../persistence/storage/remove/BlockAsDirRemover.java	82.35% <100.00%> (-17.65%)	⬇️
...er/persistence/storage/write/BlockAsDirWriter.java	100.00% <100.00%> (ø)
...istence/storage/write/BlockAsDirWriterBuilder.java	100.00% <100.00%> (ø)
...ator/exception/BlockSimulatorParsingException.java	50.00% <100.00%> (-50.00%)	⬇️
.../simulator/generator/BlockAsFileLargeDataSets.java	90.62% <100.00%> (ø)