fix!: add upgrade plugin that fixes invalid date serialisations #2081
Conversation
| @@ -90,7 +90,7 @@ WHERE { | |||
|
|
|||
| @maybeLastModificationDate match { | |||
| case Some(lastModificationDate) => { | |||
| ?resource knora-base:lastModificationDate "@lastModificationDate"^^xsd:dateTime . | |||
| ?resource knora-base:lastModificationDate ?lastModificationDate . | |||
There was a problem hiding this comment.
Is the last modification date still typed in the triplestore with this change?
There was a problem hiding this comment.
yes, I'm just using whatever is in the triplestore already rather than the value passed to he template, when deleting the old last modification date. (The idea is to make this independent of the serialization in the triplestore)
There was a problem hiding this comment.
Ah, I misread it. It's only inside the DELETE and WHERE clause, the INSERT still has the typed version! Ignore my comment then.
.../src/main/scala/org/knora/webapi/store/triplestore/upgrade/plugins/UpgradePluginPR2081.scala
Outdated
Show resolved
Hide resolved
| val statementsToRemove: collection.mutable.Set[Statement] = collection.mutable.Set.empty | ||
| val statementsToAdd: collection.mutable.Set[Statement] = collection.mutable.Set.empty |
There was a problem hiding this comment.
Looks familiarly inelegant 🤣
There was a problem hiding this comment.
I'm glad you dislike it too! xD
Makefile
Outdated
| .PHONY: init-db-test-from-ls-test-server-trig-file | ||
| init-db-test-from-ls-test-server-trig-file: init-db-test-empty ## init local database with data from a local ls-test-server dump | ||
| @echo $@ | ||
| @curl -X POST -H "Content-Type: application/sparql-update" -d "DROP ALL" -u "admin:test" "http://localhost:3030/knora-test" | ||
| @curl -X POST -H "Content-Type: application/trig" --data-binary "@${CURRENT_DIR}/db_ls_test_server_dump.trig" -u "admin:test" "http://localhost:3030/knora-test" | ||
|
|
There was a problem hiding this comment.
IMO we could have above init-db-test-from-ls-test-server make command split into 2 steps anyway
- dump the data first
- load it from that file
So basically command introduced by you stays and the other one could be reduced. Reason for that is the huge amount of data and the issue with that file if the download dies before it ends.
There was a problem hiding this comment.
That was my initial thought. (Additionally, once downloaded, you don't need the password anymore, which is nice.)
But I didn't want to mess with the other make target, so I thought I'd add a separate one. Do you think it makes a difference?
There was a problem hiding this comment.
You can leave it like that for now, I might fo it later on. I had some problems with dumping the data and then you need to remember to remove the file otherwise another error appears. This could be overall improved but not the scope of this PR.
There was a problem hiding this comment.
makes sense, thanks!
There was a problem hiding this comment.
Could you quickly add what Marcin suggested? It follows the pattern we are using in other cases.
BalduinLandolt
changed the title
BalduinLandolt
changed the title
|
Kudos, SonarCloud Quality Gate passed!
|
BalduinLandolt
deleted the
wip/DEV-1033-fix-incorrectly-serialized-date-values
branch
Resolves DEV-1033
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: DEV-1033
What is the new behavior?
Does this PR introduce a breaking change?
Major version bump is required to run the upgrade plugin.
Other information