TACKLE-312: import dependencies

[m-brophy]

https://issues.redhat.com/browse/TACKLE-312

[codecov]

Codecov Report

Merging #136 (9777d86) into main (43a47d7) will increase coverage by 0.33%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main     #136      +/-   ##
============================================
+ Coverage     97.32%   97.65%   +0.33%     
- Complexity      267      296      +29     
============================================
  Files            25       26       +1     
  Lines           448      511      +63     
  Branches         23       34      +11     
============================================
+ Hits            436      499      +63     
  Misses            9        9              
  Partials          3        3              

Impacted Files	Coverage Δ
...ioninventory/entities/ApplicationImportForCsv.java	0.00% <ø> (ø)
...plicationinventory/entities/ApplicationImport.java	100.00% <100.00%> (ø)
...tioninventory/entities/ApplicationsDependency.java	100.00% <100.00%> (ø)
...ventory/mapper/ApplicationDependencyAPIMapper.java	100.00% <100.00%> (ø)
...e/applicationinventory/services/ImportService.java	100.00% <100.00%> (ø)

---

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 43a47d7...9777d86. Read the comment docs.

[carlosthe19916]

@m-brophy Thanks for creating this PR. Here are some observations from my review:

• I've created a CSV file that generates a Circle dependency. The content of the CSV is not processed and the file imported never appears listed on Applications import page. Here is the CSV I've tried:

import-circle-dependency-generates-error.csv

• There are mainly format corrections I'd like to be done. Things like multiple white spaces or System.out.println statements can be enhanced I think.

Please let me know if you were able to reproduce the issue using the CSV attached to this comment.

[carlosthe19916]

@m-brophy this PR now works as expected. I mean, it does what is required.

Just one Enhancement I'd like to be done, and this is more about design rather than the functionality of the code. Currently, there are 2 classes that inherit ApplicationMapper; one of them is ApplicationInventoryAPIMapper and the other one is ApplicationDependencyAPIMapper. Both children classes are instantiated in a different way; ApplicationInventoryAPIMapper is instantiated with the new operator, for instance new ApplicationInventoryAPIMapper(). On the other hand, ApplicationInventoryAPIMapper is instantiated using CDI injection, for instance @Inject ApplicationInventoryAPIMapper. That seems odd.

I think the fact that both children are instantiated in a different way is a sign that perhaps both children shouldn't inherit a parent class. Moreover, it seems that the parent class is not used but only the children.

Either both children should be instantiated in the same way or both children classes should be independent and not share a common parent. Having different ways of instantiating children might create confusion.

Here is an example of 2 ways of instantiating the same class:

• https://github.com/m-brophy/tackle-application-inventory/blob/TACKLE-312/src/test/java/io/tackle/applicationinventory/resources/ApplicationImportNullTest.java#L247
• https://github.com/m-brophy/tackle-application-inventory/blob/TACKLE-312/src/main/java/io/tackle/applicationinventory/services/ImportService.java#L65

I'd like to know your thoughts about this.

[carlosthe19916]

Thanks, @m-brophy . This PR looks good to me!