Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port: Truncate component property value #748

Merged
merged 5 commits into from
Jun 20, 2024

Conversation

sahibamittal
Copy link
Collaborator

@sahibamittal sahibamittal commented Jun 18, 2024

Description

Truncates ComponentProperty values.

This is both to prevent the database's length constraint from being violated, and to avoid storing unnecessary long values. Some tools may misuse properties, and in those cases having the complete value is usually not of interest to users.

Addressed Issue

Backport change DependencyTrack/hyades#1190

Checklist

  • I have read and understand the contributing guidelines
  • This PR fixes a defect, and I have provided tests to verify that the fix is effective
  • This PR implements an enhancement, and I have provided tests to verify that it works as intended
  • This PR introduces changes to the database model, and I have updated the migration changelog accordingly
  • This PR introduces new or alters existing behavior, and I have updated the documentation accordingly

@sahibamittal sahibamittal marked this pull request as draft June 18, 2024 13:56
@sahibamittal sahibamittal marked this pull request as ready for review June 18, 2024 14:23
Copy link

codacy-production bot commented Jun 18, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for 7e4c7c11 100.00% (target: 70.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (7e4c7c1) Report Missing Report Missing Report Missing
Head commit (0f5381b) 20452 16697 81.64%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#748) 1 1 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences


🚀 Don’t miss a bit, follow what’s new on Codacy.

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

Footnotes

  1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

@nscuro
Copy link
Member

nscuro commented Jun 18, 2024

The limit should also apply to the database schema:

<createTable tableName="COMPONENT_PROPERTY">
<column autoIncrement="true" name="ID" type="BIGINT">
<constraints nullable="false" primaryKey="true" primaryKeyName="COMPONENT_PROPERTY_PK"/>
</column>
<column name="COMPONENT_ID" type="BIGINT">
<constraints nullable="false"/>
</column>
<column name="GROUPNAME" type="TEXT"/>
<column name="PROPERTYNAME" type="TEXT">
<constraints nullable="false"/>
</column>
<column name="PROPERTYVALUE" type="TEXT"/>
<column name="PROPERTYTYPE" type="TEXT">
<constraints nullable="false"/>
</column>
<column name="DESCRIPTION" type="TEXT"/>
<column name="UUID" type="TEXT">
<constraints nullable="false"/>
</column>
</createTable>

On the same note, could you also add the limits for the other fields so they're in sync with what's defined on the Java model?

@sahibamittal
Copy link
Collaborator Author

sahibamittal commented Jun 18, 2024

The limit should also apply to the database schema:

On the same note, could you also add the limits for the other fields so they're in sync with what's defined on the Java model?

Sure, though fields other than propertyValue don't have length constraints in java model, do you want me add them like in upstream?

@nscuro
Copy link
Member

nscuro commented Jun 18, 2024

Argh, apologies I was looking at the upstream code indeed. I think it makes sense to add them for consistency, WDYT?

@nscuro nscuro added this to the 5.5.0 milestone Jun 18, 2024
@nscuro nscuro added enhancement New feature or request v4-port PRs that were ported from the Dependency-Track v4.x code base labels Jun 18, 2024
nscuro
nscuro previously approved these changes Jun 18, 2024
@nscuro
Copy link
Member

nscuro commented Jun 18, 2024

Seems the new SARIF test is a bit flaky:

 Error:  Failures: 
Error:    FindingResourceTest.getSARIFFindingsByProjectTest:695 JSON documents are different:
Different value found in node "runs[0].results[1].level", expected: <"note"> but was: <"error">.
Different value found in node "runs[0].results[1].message.text", expected: <"A description-with-hyphens-(and parentheses)"> but was: <"Yet another description but with surrounding whitespaces">.
Different value found in node "runs[0].results[1].properties.cweId", expected: <"23"> but was: <"46">.
Different value found in node "runs[0].results[1].properties.recommendation", expected: <"Recommendation with whitespaces"> but was: <"">.
Different value found in node "runs[0].results[1].properties.severityRank", expected: <"3"> but was: <"1">.
Different value found in node "runs[0].results[1].ruleId", expected: <"Vuln-3"> but was: <"Vuln-2">.
Different value found in node "runs[0].results[2].level", expected: <"error"> but was: <"note">.
Different value found in node "runs[0].results[2].message.text", expected: <"Yet another description but with surrounding whitespaces"> but was: <"A description-with-hyphens-(and parentheses)">.
Different value found in node "runs[0].results[2].properties.cweId", expected: <"46"> but was: <"23">.
Different value found in node "runs[0].results[2].properties.recommendation", expected: <""> but was: <"Recommendation with whitespaces">.
Different value found in node "runs[0].results[2].properties.severityRank", expected: <"1"> but was: <"3">.
Different value found in node "runs[0].results[2].ruleId", expected: <"Vuln-2"> but was: <"Vuln-3">.

Do you see any way we can stabilize it?

@nscuro nscuro merged commit d4919a4 into main Jun 20, 2024
8 checks passed
@nscuro nscuro deleted the port-truncate-ComponentProperty-value branch June 20, 2024 14:26
@nscuro nscuro changed the title Port : Truncate component property value Port: Truncate component property value Jun 24, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request v4-port PRs that were ported from the Dependency-Track v4.x code base
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants