Move file handling logic from RecordItemParser to RecordFileParser

[apeksharma]

Detailed description:

• Move connection ownership to PostgresWritingRecordParsedItemHandler
  • RecordItemParser.start()+ RecordItemParser.initFile --> onStart()
  • RecordItemParser.finish + RecordItemParser.completeFile --> onEnd()
• RecordItemParser is agnostic of 'file' and database now (except entity repo, to be fixed later)
• RecordFileParser is agnostic of database now (except for application state - applicationStatusRepository)
• Change RecordFileParserTest to use mocks rather than transactionRepository or recordFileRepository.
• Adds two interfaces: StreamFileListener and RecordStreamFileListener
• Dependencies:
  • RecordFileParser ---depends on --> RecordStreamFileListener and RecordItemListener
  • RecordItemParser --depends on--> RecordParsedItemHandler
• Deprecated file id column in t_record_files. To be removed soon.
• Remove checks for recordFileRepository from RecordItemParser tests

Signed-off-by: Apekshit Sharma [email protected]

Which issue(s) this PR fixes:
Partially addresses #566

Special notes for your reviewer:

Checklist

• Documentation added
• Tests updated

[codecov]

Codecov Report

Merging #585 into master will increase coverage by 0.14%.
The diff coverage is 76.47%.

@@             Coverage Diff              @@
##             master     #585      +/-   ##
============================================
+ Coverage     64.31%   64.45%   +0.14%     
  Complexity      171      171              
============================================
  Files           101      101              
  Lines          3167     3157      -10     
  Branches        366      363       -3     
============================================
- Hits           2037     2035       -2     
+ Misses          976      971       -5     
+ Partials        154      151       -3

Impacted Files	Coverage Δ	Complexity Δ
...com/hedera/mirror/importer/domain/Transaction.java	82.6% <ø> (ø)	0 <0> (ø)	⬇️
...importer/parser/record/RecordParserProperties.java	92.85% <ø> (-0.48%)	0 <0> (ø)
.../com/hedera/mirror/importer/domain/RecordFile.java	66.66% <100%> (+9.52%)	0 <0> (ø)	⬇️
...porter/parser/record/PostgresWriterProperties.java	66.66% <100%> (+16.66%)	0 <0> (ø)	⬇️
...irror/importer/parser/record/RecordItemParser.java	83.89% <100%> (+1.3%)	0 <0> (ø)	⬇️
...irror/importer/parser/record/RecordFileParser.java	73.05% <72.5%> (+5.79%)	0 <0> (ø)	⬇️
...record/PostgresWritingRecordParsedItemHandler.java	82.56% <74%> (-2.87%)	0 <0> (ø)
...hedera/mirror/importer/util/DatabaseUtilities.java	35.71% <0%> (-21.43%)	0% <0%> (ø)
...r/grpc/retriever/PollingTopicMessageRetriever.java	96% <0%> (-2%)	12% <0%> (ø)
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac0f386...57da8b3. Read the comment docs.

[steven-sheehy]

I don't agree with the direction of this PR. The RecordFileParser should not have knowledge of database connections, rolling back or postgres specific parsed item handlers.

Recommend moving connection management to PostgresWritingRecordParsedItemHandler and be opened once per file. It can even be opened once per batch due to the wonders of connection pools. It's fine if RecordFileParser needs to open a separate connection for updating t_record_files. Recommend interface segregation principle be used to split file specific logic out from ParsedItemHandler and into new interface (exact name or parameters can be debated):

public interface StreamFileListener {
  void onStart(StreamFileData);
  void onEnd(StreamFileData);
}

PostgresWritingRecordParsedItemHandler should be adapted to implement StreamFileListener directly (not via ParsedItemHandler) only because he manually manages connections and needs to flush onEnd. Other implementations of ParsedItemHandler like message queues would not need to implement it as they don't batch.

RecordFileParser should only have knowledge of StreamFileListener and RecordItemParser should only have knowledge of ParsedItemHandler. This would provide a much cleaner separation of layers while still satisfying your concerns.

[apeksharma]

I absolutely agree, don't like the postgres stuff in two places myself.
Will use suggestions above to clean it up.
Please ignore ugliness around fileId in current PR since it's going to be removed in followup.

[apeksharma]

last commit was early preview of changes, please take a look.
Am working on fixing tests in meantime.

[apeksharma]

improvement suggestions would be great! There's still so much here (and in general, always), that can be made better.
But if may request, since this PR has grown big and iterations on this will be costly, let's do improvements in followup. There's lot of good stuff here already. :)