[PB-557] bug: file size from the drive-server comes as string

[edisonjpadilla]

Added the virtual field numericSize in order to return the file size as a number instead of string. Whichever client needs the size as a number could simply use this property.

[JoanVicens]

Tested and works, but why do we parser to a float? The size is in bytes so it shuld be always an integer

[edisonjpadilla]

@JoanVicens I thought the size was measured in MB, I'll make the change.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

[JoanVicens]

@PixoDev Can this break anything on mac?

[PixoDev]

This will break a lot of API responses in the MacOS app because we are expecting an string there @JoanVicens @edisonjpadilla.

Let me think a solution for this so we don't break production already please, don't merge this yet. Will this apply to all the responses that includes the size field?

[JoanVicens]

Added a new review to block the merge button. We need to ensure this will not break any client

[JoanVicens]

We could add a new field so the clients can migrate to it and once every client no longer uses it change the size to number (and migrate again?) @sg-gs

[jzunigax2]

We could add a new field so the clients can migrate to it and once every client no longer uses it change the size to number (and migrate again?) @sg-gs

@JoanVicens I was taking a look into this old issue, is this approach still needed?

[JoanVicens]

We could add a new field so the clients can migrate to it and once every client no longer uses it change the size to number (and migrate again?) @sg-gs

@JoanVicens I was taking a look into this old issue, is this approach still needed?

Yes, I believe that the parsing of the JSON on the Mac application relies on the type of the fields being what they expect, so changing the size to a number would be a breaking change.

[apsantiso]

This solution is the way to go if we aim for non breaking changes, but it introduces technical debt. Specifically, you would need to replicate this approach in the drive-server-wip repository (you could add it to the domain layer instead of using a virtual value). Moreover, if we decide to change the size to numeric, the client currently using this virtual value would need to switch back to using size.

This issue originates from Sequelize, where the size ( in the DB ) column is of type "bigint". Sequelize cannot ensure that very large numbers are safely parsed to Number, so it parses this column as a String for such cases. However, the MAX SAFE INTEGER in JavaScript is close to 8000TB (9007199254740991 in bytes), so we can safely parse this to Number without any problems if needed.

@sg-gs @jzunigax2 Feel free to merge this, but I believe such issues should be addressed at the root level. Instead of adding more fields, we could have the macOS client handle either strings or numbers if possible.

I mention this because I've also had issues with this field being of string type on drive-server-wip while building the workspaces feature (when comparing size < free space, for instance)

[sonarqubecloud]

Quality Gate failed

Failed conditions
1 Security Hotspot
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud