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

Add tracelocation byterepresentation to TraceLocation database table (DEV) #2647

Merged

Conversation

LukasLechnerDev
Copy link
Contributor

@LukasLechnerDev LukasLechnerDev commented Mar 18, 2021

TechSpec says to store the byte representation of the TraceLocation into the TraceLocation database table.

This PR does exactly that.

We decided to store the byte representation as base64 encoded string in the database.

@LukasLechnerDev LukasLechnerDev added the maintainers Tag pull requests created by maintainers label Mar 18, 2021
@LukasLechnerDev LukasLechnerDev added this to the 2.0.0 milestone Mar 18, 2021
@LukasLechnerDev LukasLechnerDev requested a review from a team March 18, 2021 16:16
@LukasLechnerDev LukasLechnerDev marked this pull request as draft March 18, 2021 16:29
…epresentation

# Conflicts:
#	Corona-Warn-App/schemas/de.rki.coronawarnapp.eventregistration.storage.TraceLocationDatabase/1.json
@LukasLechnerDev LukasLechnerDev force-pushed the feature/DEV-Add-tracelocation-byterepresentation branch from b3b60f7 to fda4cba Compare March 22, 2021 12:49
@LukasLechnerDev LukasLechnerDev marked this pull request as ready for review March 22, 2021 12:51
@LukasLechnerDev LukasLechnerDev changed the title Add tracelocation byterepresentation to TraceLocation and CheckIn (DEV) Add tracelocation byterepresentation to TraceLocation database table (DEV) Mar 22, 2021
@@ -18,8 +18,8 @@ data class TraceLocationEntity(
@ColumnInfo(name = "startDate") val startDate: Instant?,
@ColumnInfo(name = "endDate") val endDate: Instant?,
@ColumnInfo(name = "defaultCheckInLengthInMinutes") val defaultCheckInLengthInMinutes: Int?,
@ColumnInfo(name = "byteRepresentation") val byteRepresentationBase64: String,
Copy link
Member

Choose a reason for hiding this comment

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

Should the columnName then also be byteRepresentationBase64 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes makes sense. However we are also not suffixing the base64 encoded values in the CheckInEntity. I think a PR in which we refactor the 2 entities would be best. I can do this after this one is merged, if that's fine for you?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good.

@chiljamgossow chiljamgossow self-assigned this Mar 22, 2021
@d4rken d4rken self-assigned this Mar 23, 2021
@sonarcloud
Copy link

sonarcloud bot commented Mar 23, 2021

Kudos, SonarCloud Quality Gate passed!

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

80.0% 80.0% Coverage
0.0% 0.0% Duplication

@d4rken d4rken merged commit 653ce1f into release/2.0.x Mar 23, 2021
@d4rken d4rken deleted the feature/DEV-Add-tracelocation-byterepresentation branch March 23, 2021 09:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
maintainers Tag pull requests created by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants