Snippets orphan removal #2146
Snippets orphan removal #2146
Conversation
kamil-bielecki-bosch
requested review from
bs-ondem,
MarcelBochtler,
mnonnenmacher,
nnobelis,
oheger-bosch,
sschuberth and
wkl3nk
as code owners
There was a problem hiding this comment.
One general remark: Scan results produced by ScanCode are shared between multiple ORT runs, so it makes sense to handle them by the orphan service.
For snippet results, this is not the case; they are exclusively assigned to a single ORT run. (Or in other words: For every ORT run that has snippet scanning enabled, another scan is started, and the results are stored.) So, would it be better to directly delete snippet results (if they exist) when the owning ORT run is deleted?
| @@ -23,75 +23,58 @@ import io.kotest.core.spec.style.WordSpec | |||
| import io.kotest.matchers.shouldBe | |||
There was a problem hiding this comment.
Commit message: Please provide a rationale why this extraction is done.
| ) | ||
|
|
||
| // More orphan entries - should be deleted by removal process | ||
| createVcsInfoTableEntry(url = "to.delete3") | ||
|
|
||
| VcsInfoTable.selectAll().count() shouldBe 11 | ||
| VcsInfoTable.selectAll().count() shouldBe 17 |
There was a problem hiding this comment.
Why do some assertions have to be changed? This should not be the case if the fixtures were just moved to a separate class, right?
There was a problem hiding this comment.
I've extended commit message for this commit.
Basically - Fixtures were moved to another class, and aligned that every fixture has full list of fields of table that is covering. That caused need to align tests accordingly.
| * Default, unique values are generated for every field, to provide entry uniqueness. | ||
| * REMARK: The functions are not creating it's own transactions, so should be used in db.dbQuery { ... } blocks. | ||
| */ | ||
| internal object TableEntryTestFixtures { |
There was a problem hiding this comment.
The name TableEntryTestFixtures is rather unspecific. Should it be named something like OrphanRemovalTestFixtures?
| * Set of Database fixtures for testing purposes. | ||
| * Every function creates a new entry in the corresponding table and returns the id of the created entry. | ||
| * Default, unique values are generated for every field, to provide entry uniqueness. | ||
| * REMARK: The functions are not creating it's own transactions, so should be used in db.dbQuery { ... } blocks. |
There was a problem hiding this comment.
"... their own transactions..."
| id notInSubQuery ( | ||
| SnippetFindingsSnippetsTable | ||
| .select(SnippetFindingsSnippetsTable.snippetId.alias("id")) | ||
| .where(SnippetFindingsSnippetsTable.snippetId.isNotNull()) |
There was a problem hiding this comment.
This condition looks strange. Are you sure it is correct? Are snippets not handled by cascade rules?
There was a problem hiding this comment.
SnippetFindingsSnippetsTable is an connection table between snippet_findings and snippets table. So, as far as Snippet can be referenced by more than one scan (at least from DB perspective) it's not safe to delete snippets by cascade delete.
@oheger-bosch Is it true ? I remember in the past, snippets were assigned to the scan results, and a ORT run could have several scan results because of duplication of them. |
No, this is not what I am saying. What I say is that a snippet result does only belong to a single ORT run, it is not shared between multiple runs. It might be the case that this ORT run has multiple snippet results though. |
kamil-bielecki-bosch
force-pushed
the
snippets-orphan-removal
branch
7 times, most recently
from
7be405e to
ec2e51c
Compare
|
@oheger-bosch So, in that case, there should be just extension on cascade deletes from ort-runs up to at least |
kamil-bielecki-bosch
force-pushed
the
snippets-orphan-removal
branch
2 times, most recently
from
9542b49 to
e67c030
Compare
To prevent test class further growth, table fixtures used are extracted to external class. To provide flexibility, method parameter lists are extended, to cover full list of fields for every table. Tests using fixtures are aligned to new parameter lists. Signed-off-by: Kamil Bielecki <[email protected]>
During deletion of old / outdated ORT runs some of child database entities were left in DB. To prevent this situation, that leads to database performance issues, orphaned records are deleted. Deleted record types: - Scan results - Scan summaries - Snippet findings - Snippets - License findings - Copyright findings Signed-off-by: Kamil Bielecki <[email protected]>
kamil-bielecki-bosch
force-pushed
the
snippets-orphan-removal
branch
from
e67c030 to
5e03ad8
Compare
|
Obsolete, due to design changes |
No description provided.