DB: Fix 0/NULL issues when reading source_synchronized_ms

[uklotzde]

Unfortunately I noticed a very strange behavior of Qt when reading a nullable integer database column that contains NULL:

// Observation (Qt 5.15): QVariant::isValid() may return true even
// if the column value is NULL and then converts to 0 (zero). This
// is NOT desired and therefore we MUST USE QVariant::isNull() instead!

As a consequence the column library.source_synchronized_ms now contains 0 (= zero) instead of NULL for some tracks after they have been loaded and saved at least once after the migration.

Do we need a formal DB schema update to fix this issue? This would probably be the safe solution.

The first commit is just a minor logging improvement to detect inconsistencies between the library and external files.

[uklotzde]

I have added the schema migration.

For extra safety the value 0 is no longer valid, because it indicates a bogus value.

[Holzhaus]

For extra safety the value 0 is no longer valid, because it indicates a bogus value.

What if I last modified my file on January 1st, 1970?😁

Seriously though, from time to time I've seem those kind of file modification times, probably because some other software was buggy. Would that cause any undesired behavior?

[poelzi]

All files in the nix store are defaulted to that modified time if you don't set it to something specific

-r-xr-xr-x 1 root root 1495112 Jan  1  1970 /nix/store/lz515dc98y93x2srr0gq50zzsiwbffh8-sqlite-3.33.0-bin/bin/sqlite3

It could be that somebody nixifies his music collection ;)

[uklotzde]

All files in the nix store are defaulted to that modified time if you don't set it to something specific

-r-xr-xr-x 1 root root 1495112 Jan  1  1970 /nix/store/lz515dc98y93x2srr0gq50zzsiwbffh8-sqlite-3.33.0-bin/bin/sqlite3

It could be that somebody nixifies his music collection ;)

Just one more reason to ignore this time stamp and treat it as NULL, i.e. unknown.

[uklotzde]

If the file modification time doesn't reflect a valid time then it should be ignored. That's what this PR does besides fixing the actual bug.

[uklotzde]

It would be dangerous if we add logic on top of useless data. If you don't time stamp your files properly you will simply not be able to use the extended metadata synchronization. That's safe and fair.

[uklotzde]

I must admit that I didn't think about the 0 edge case. In the end the occurrence of this bug was actually helpful to discover and resolve it early before it could do any harm 😅

[uklotzde]

@mr-smidge I fixed a possible edge case

[mr-smidge]

Nice, thank you 👍

[daschuer]

LGTM, Thank you.