Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Fix certificate highest priority fallback behavior (DEV) #3845

Merged

Conversation

d4rken
Copy link
Member

@d4rken d4rken commented Jul 30, 2021

Fixes return the wrong certificate if we have no only expired/invalid ones.

If no certificates matches a rule, we return null.
Since the introduction of the states, we process them in the order Valid/Expiring>Expired>Invalid.
When we have listOf(emptyList(), emptyList(), list(cert)) we would get listOf(null,null,cert).
As our fallback behavior is list.firstOrNull() ?: fallback it would always trigger the fallback if the first list (valid/expiring) is empty.

Prevent fallback behavior from triggering early if there are no certificate with `Valid` state.
@d4rken d4rken added bug Something isn't working maintainers Tag pull requests created by maintainers labels Jul 30, 2021
@d4rken d4rken added this to the 2.7.0 milestone Jul 30, 2021
@d4rken d4rken requested a review from a team July 30, 2021 11:37
Comment on lines +178 to +197

/**
* Bad: listOf(null,null,cert).map {...}.firstOrNull() ?: fallback
*
* findHighestPriorityCertificate(nowUtc=2021-06-24T14:00:00.000Z): [VaccinationCertificate(#1), VaccinationCertificate(#7)]
* No certs with state Valid/ExpiringSoon
* Checking 2 certs with for Expired
* Rule 3 match (Series-completing Vaccination Certificate > 14 days): VaccinationCertificate(#7)
* No certs with state Invalid
* No priority match, this should not happen: [VaccinationCertificate(#1), VaccinationCertificate(#7)]
*
*
* Good: listOf(null,null,cert).mapNotNull {...}.firstOrNull() ?: fallback
*
* findHighestPriorityCertificate(nowUtc=2021-06-24T14:00:00.000Z): [VaccinationCertificate(#1), VaccinationCertificate(#7)]
* No certs with state Valid/ExpiringSoon
* Checking 2 certs with for Expired
* Rule 3 match (Series-completing Vaccination Certificate > 14 days): VaccinationCertificate(#7)
* No certs with state Invalid
*/
Copy link
Member Author

Choose a reason for hiding this comment

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

Example

Copy link
Contributor

@BMItr BMItr left a comment

Choose a reason for hiding this comment

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

lgtm

@BMItr BMItr self-assigned this Jul 30, 2021
@mtwalli mtwalli self-assigned this Jul 30, 2021
@sonarcloud
Copy link

sonarcloud bot commented Jul 30, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

94.4% 94.4% Coverage
0.0% 0.0% Duplication

@mtwalli mtwalli merged commit c4c9df8 into release/2.7.x Jul 30, 2021
@mtwalli mtwalli deleted the fix/DEV-certificate-highest-priority-fallback-behavior branch July 30, 2021 14:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working maintainers Tag pull requests created by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants