[TACKLE-268]-Request failed with status 500 on importing invalid csv file

[carlosthe19916]

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

About creating SQL scripts to change the column sizes

The current definition of the table application_import is something like:

create table application_import (
    id int8 not null,
    businessService varchar(255),
    description varchar(255),
    applicationname varchar(255),
    comments varchar(255),
    recordType1 varchar(255),
    tag1 varchar(255),
    tagType1 varchar(255),
    tag2 varchar(255),
    tagType2 varchar(255),
    tag3 varchar(255),
    tagType3 varchar(255),
    tag4 varchar(255),
    tagType4 varchar(255),
    errorMessage varchar(255),
    isValid boolean,
    primary key (id)
)

In our last meeting, it was suggested to create a new SQL script to alter the application_import table and align the column sizes to the UI; it means, to reduce some column's size. For instance, applicationname varchar(255) will become applicationname varchar(120) . This change will work in an empty database but it will cause problems if for any reason the DB is not empty and the table application_import contains applicationname column values longer than 120.

[codecov]

Codecov Report

Merging #107 (696ae60) into main (181b593) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main     #107      +/-   ##
============================================
+ Coverage     97.27%   97.32%   +0.04%     
- Complexity      219      220       +1     
============================================
  Files            25       25              
  Lines           441      448       +7     
  Branches         22       23       +1     
============================================
+ Hits            429      436       +7     
  Misses            9        9              
  Partials          3        3              

Impacted Files	Coverage Δ
...plicationinventory/entities/ApplicationImport.java	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 181b593...696ae60. Read the comment docs.

[mrizzi]

The validation approach has been widely discussed on some meetings and the implementation has been done accordingly so no need to further discuss the validation approach in this PR.
I've reviewed the test class in order to enhance where possible.
I'm aware of the need for quickly get this PR merged so I'll provide a PR to this PR to fix all the comments I raised and also for adding the native test once only HTTP endpoints are used in the test.

[mrizzi]

There's just one test method so no need to use @Order(X) annotation.

Suggested change

@TestMethodOrder(MethodOrderer.OrderAnnotation.class)

[mrizzi]

Better keep the naming consistent with the other issue-related test class IssueTACKLE282Test and add the link to the issue for a quick reference in case of need.

Suggested change

	public class Issue268Test extends SecuredResourceTest {
	// https://issues.redhat.com/browse/TACKLE-268
	public class IssueTACKLE268Test extends SecuredResourceTest {

[mrizzi]

Here we can leverage and be consistent with the usage of the WireMockControlsServices recently added that allows for removing this code.

[mrizzi]

Relying only on REST endpoints makes it easier for using the test also in native mode.

[mrizzi]

From above, we can leverage and be consistent with the usage of the WireMockControlsServices recently added that allows for removing this code.

[mrizzi]

It's a call to the same /import-summary tested in the call straight above so all the tests can be done with just one call.

[mrizzi]

The summary ID can be retrieve from the above test call to the /import-summary endpoint avoiding the need for executing this query.

[mrizzi]

importSummaryId should be managed properly as a query parameter

[mrizzi]

using the summary ID retrieve in a call above, this transaction can be replaced with a REST delete call.

[mrizzi]

This is tested in the above RESTAssured call.

Suggested change

assertEquals(200, response.getStatusCode());

[mrizzi]

As already reported, validation process already discussed outside of this PR.
The test part of the PR has been reviewed so I think we're fine to merge this.
The PR has been also manually tested in a live Tackle instance (thanks @PhilipCattanach)

I approve even if I've been a contributor but we need this to let #92 build successfully.

Thanks @carlosthe19916 for having done this enhancement 👍