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

Refactoring security-analysis-api : Remove WithLog suffixed methods #1667

Merged

Conversation

tadam50
Copy link
Contributor

@tadam50 tadam50 commented Mar 24, 2021

Please check if the PR fulfills these requirements (please use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes)

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem ? If so, link to this issue using '#XXX' and skip the rest
fix #1602

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Refactoring of security-analysis-api module

What is the current behavior? (You can also link to an open issue here)
Results are represented by two différents classes SecurityAnalysisResult & SecurityAnalysisResultWithLog.
SecurityAnalysisProvider provides two différents methods run & runWithLog

What is the new behavior (if this is a feature change)?
SecurityAnalysisResultWithLog is removed and logBytes field is forwarded into SecurityAnalysisResult.
SecurityAnalysis, SecurityAnalysisProvider & SecurityAnalysisExecution do not contains WithLog suffixed methods.
A new field boolean withLogs was added into SecurityAnalysisExecutionInput class representing "log-file" SecurityAnalysisTool option.
This new field in SecurityAnalysisExecution implementations, to know if log files is requested.

Does this PR introduce a breaking change or deprecate an API? If yes, check the following:

  • The Breaking Change or Deprecated label has been added
  • The migration guide has been updated in the github wiki (What changes might users need to make in their application due to this PR?)

Other information:

(if any of the questions/checkboxes don't apply, please delete them entirely)

tadam50 added 2 commits March 23, 2021 17:58
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Copy link
Contributor

@sylvlecl sylvlecl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be discussed : I am not sure that including the logs in the result will be the best solution.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 1, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

91.8% 91.8% Coverage
0.0% 0.0% Duplication

@miovd miovd requested a review from sylvlecl April 19, 2021 07:43
@miovd miovd merged commit b6a6bd3 into integration/refactoring-security-analysis-api May 4, 2021
@miovd miovd deleted the as_refactoring_issue_1602 branch May 4, 2021 12:40
miovd pushed a commit that referenced this pull request May 4, 2021
…1667)

* Remove SecurityAnalysisResultWithLog class
* Replace WithLog methods with bollean usage
* Add SecurityAnalysisReport composed by SecurityAnalysisResult & LogBytes

Signed-off-by: Thomas ADAM <tadam@silicom.fr>
miovd pushed a commit that referenced this pull request May 4, 2021
…1667)

* Remove SecurityAnalysisResultWithLog class
* Replace WithLog methods with bollean usage
* Add SecurityAnalysisReport composed by SecurityAnalysisResult & LogBytes

Signed-off-by: Thomas ADAM <tadam@silicom.fr>
miovd pushed a commit that referenced this pull request May 25, 2021
…1667)

* Remove SecurityAnalysisResultWithLog class
* Replace WithLog methods with bollean usage
* Add SecurityAnalysisReport composed by SecurityAnalysisResult & LogBytes

Signed-off-by: Thomas ADAM <tadam@silicom.fr>
EtienneLt pushed a commit that referenced this pull request May 27, 2021
…1667)

* Remove SecurityAnalysisResultWithLog class
* Replace WithLog methods with bollean usage
* Add SecurityAnalysisReport composed by SecurityAnalysisResult & LogBytes

Signed-off-by: Thomas ADAM <tadam@silicom.fr>
miovd pushed a commit that referenced this pull request Jun 14, 2021
…1667)

* Remove SecurityAnalysisResultWithLog class
* Replace WithLog methods with bollean usage
* Add SecurityAnalysisReport composed by SecurityAnalysisResult & LogBytes

Signed-off-by: Thomas ADAM <tadam@silicom.fr>
miovd added a commit that referenced this pull request Jun 14, 2021
* Remove deprecated code (#1579)
* Use ServiceLoader to create SecurityAnalysis (#1585)
* Add SecurityAnalysisProvider
* Add FIXME to rename later and fix comments issues
* Remove runWithLog method in SecurityAnalysisProvider
* Replace SecurityAnalysis factory uses by loaded SecurityAnalysisProvider (#1618)
* Refactoring security-analysis-api : SecurityAnalysisImpl (issue 1604) (#1648)
* Refactoring security-analysis-api : Remove WithLog suffixed methods (#1667)
* Remove SecurityAnalysisResultWithLog class
* Replace WithLog methods with bollean usage
* Add SecurityAnalysisReport composed by SecurityAnalysisResult & LogBytes

Signed-off-by: Thomas ADAM <tadam@silicom.fr>

* After rebasing: bump to 4.2.0-SNAPSHOT version + do not use "default" word in automatic module name (forbidden)
* Add default impl of API AS in distribution core
* Bump to 4.3.0-SNAPSHOT

Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>

* Monitor security analysis (#1737)
* On demand results for specific equipments

Signed-off-by: Etienne Lesot <etienne.lesot@rte-france.com>

* Corrections following reviewer's comments:
- Rename SecurityAnalysisResult.getPreContingencyLimitViolationsResult
- Use only pre/post contingency in javadoc
- Put methods in StateMonitorJson in StateMonitor
- Clear before addAll in setMonitors
* Corrections following reviewer's comments:
- PreContingencyResult constructor requires Collections, not Maps
- Correct SecurityAnalysisResult deserializer and test files
- Optimize monitor merging in StateMonitorIndex
- Add ID-based getters in PreContingencyResult
- Delete redundant methods in SecurityAnalysisResult
- If a monitored ID does not exist, ignore it
- Add tests
* Rebasing from master and resolving conflicts


Co-authored-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Co-authored-by: EtienneLt <32468651+EtienneLt@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants