OPUS4 / framework

OPUS 4 Database Implementation
Other
2 stars 7 forks source link

Datumsangaben nicht als Zeitstempel speichern #273

Open j3nsch opened 2 years ago

j3nsch commented 2 years ago

Für alle Datumsfelder wird die Klasse Opus\Date verwendet. Es ist unklar warum diese Implementation gewählt wurde. Die Behandlung von Datumsangaben als Zeitstempel macht aber immer wieder die Verarbeitung kompliziert und unübersichtlich.

Wenn ein Dokument mit dem Publish-Formular anlegt, werden sämtliche Datumsangaben als String mit Zeit, als Zeitstempel, in der Datenbank gespeichert. Setzt man hingegen einen Datumswert mit PHP Code wird nur der Datumsstring gespeichert.

DB Column Mit Publish-Modul Erwartet
completed_date 2022-07-01T00:00:00+02:00 2022-07-01
thesis_date_accepted 2021-09-16T00:00:00+02:00 2021-09-16
published_date 2022-03-21T00:00:00+01:00 2022-03-21
server_date_created 2022-07-01T16:36:05+02:00 2022-07-01T16:36:05+02:00
server_date_modified 2022-07-01T16:37:36+02:00 2022-07-01T16:37:36+02:00
embargo_date 2023-01-01T00:00:00+01:00 2023-01-01

Datumsangaben sollten auch nur als Datum in der Datenbank gespeichert werden. Es kann sein, dass dieses Problem, bei Änderungen an der Date-Klasse eingeführt wurde, die notwendig waren, um andere Probleme zu beheben und zuverlässige Vergleiche zwischen Datumsangaben und Zeitstempel zu ermöglichen.

Es muss geprüft werden warum sich das Verhalten geändert hat bzw. ob es am Publish-Modul liegt und das Verhalten dort schon immer so war. Wie wird die Date-Klasse vom Publish-Modul verwendet?

Es muss überlegt werden, ob mit dem nächsten Update eine automatische Bereinigung durchgeführt werden soll. Offensichtlich gibt es auch noch Lücken in den Unit Tests.

j3nsch commented 2 years ago

@alw-bsz Es sollten Stichproben in gehosteten Instanzen gemacht werden, um zu schauen wie die Datumsangaben dort gespeichert werden. Es wäre Interessant zu sehen, ob das Format mit OPUS 4.7 gewechselt hat und ob es Unterschiede zwischen Publish und Sword erzeugten Dokumenten gibt.

Das Problem ist aufgefallen, weil die Date-Klasse gerade vom Framework nach Common verschoben wird. In Common ist das Test Setup anders und deshalb wird dort UTC verwendet. Damit sind ein paar Tests nicht durchgelaufen, die bisher davon abhängig sind in der "+02:00" Zeitzone zu laufen. Insgesamt sorgt der Umgang mit Datumsangaben und Zeitstempeln im Code immer wieder für Verwirrungen. Das gewünschte Verhalten muss ausführlich dokumentiert werden.

j3nsch commented 2 years ago

Die Datumsangaben funktionieren im aktuellen OPUS 4, weil die Date-Klasse, die Angaben entsprechend umwandelt. Datumsangaben sollten trotzdem nicht als Zeitstempel gespeichert werden, weil das zu Verwirrungen führen kann. Wir brauchen Unit Tests, die die Date Klasse umgehen, und direkt prüfen was in die Datenbank geschrieben wurde.

Im neuen Datenmodell sollten die Felder evlt. als Datum oder Zeitstempel markiert werden, so dass die Behandlung, unabhängig vom Speicherformat klar ist.

j3nsch commented 2 years ago

Beim Update müsste man die Strings für die Datumsfelder kürzen. Geht das evtl. mit SQL? SQL wäre schneller, funktioniert aber nur wenn der Datumsteil korrekt ist und nicht durch die Zeitzone "verschoben" wurde. Mit "verschoben" ist die Änderung des Datums durch die Umwandlung von der lokalen Zeitzone zu UTC gemeint. Das sollte eigentlich nicht der Fall sein, aber 100% sicher bin ich mir nicht.

j3nsch commented 2 years ago

Das Hosting beim KOBV hat bestätigt, dass in den Datumsfeldern Zeitstempel auftauchen. Ich werde mal sehen, ob sich das mit einem Update Skript bei 4.7.2 fixen lässt. Ab 4.8 werden die Datumsfeldern dann auch explizit als Datumsfelder markiert. Momentan werden alle Zeitfelder gleich behandelt und der Unterschied entsteht daraus wie man die Date-Klasse initialisiert, als Datum oder als Zeitstempel. Seit 4.7 scheint dort aber zumindest im Publish-Code etwas schief zu laufen, wofür es keinen Test gab. Daher ist es nicht aufgefallen.

j3nsch commented 2 years ago

Beispiele für Datumsangaben in den Datenbanken.

1998-07-22
2010-2-5T0:00:00CET
2003-6-2T0:00:00CEST
2002-8-10T0:00:00CEST
1999-01-12T00:00:00CET
2000-07-05T00:00:00CEST
2016-02-01T00:00:00+01:00
2017-07-27
2016-05-04T00:00:00CEST
2014-08-27T14:45:19+02:00

Bei server_date_modified taucht auch folgendes auf.

2010-06-04T02:30:32Z
2012-01-03T15:06:40+01:00
j3nsch commented 2 years ago

Das mal +01:00 und mal +02:00 auftaucht, liegt vermutlich an Sommer und Winterzeit. Die Angaben mit T0:00:00 oder T00:00:00 sollten Datumsangaben sein, bei denen der Zeitteil automatisch hinzugefügt wurde, weil das Date-Objekt nicht als isDateOnly markiert war. Das kann an einem Bug bei der Erzeugung des Date-Objekts liegen.

https://github.com/OPUS4/framework/blob/70fb1462af6f33cac78d11f45420a9eff07675fd/library/Opus/Date.php#L372

Für diese Datumsangaben sollte es möglich sein, den gespeicherten String einfach zu kürzen.

j3nsch commented 2 years ago

Wenn ein Date-Objekt mit einem Zend_Date-Objekt initialisiert wird, dann sind die Zeit-Felder auf 0 gesetzt, aber nicht auf NULL. Die isDateOnly prüft auf NULL und daher werden diese Angaben dann als Zeitstempel gespeichert. Auf 0 kann nicht geprüft werden, weil das eine gültige Zeitangabe ist. Bei der Erzeugung des Date-Objektes muss klar sein, ob es ein Datum oder ein Zeitstempel ist. In Zukunft sollte das für die Felder im Datenmodell technisch vorgegeben sein. Im Issue OPUS4/application#570 wird die Verwendung von Zend_Date auch aus der Application entfernt.

j3nsch commented 2 years ago

Ich habe die Beispielwerte oben man in einen Test gepackt, der den Code für die Anzeige verwendet. Die Datumsangaben werde unverändert angezeigt, also nicht durch die Zeitzone verschoben. Ein Bereinigung durch kürzen der Strings sollte ausreichen. Das sollte mit der SQL Funktion substring möglich sein. Allerdings muss man aufpassen, da die Datumsangaben nicht immer gleich lang sind. Manchmal werden keine führenden Nullen verwendet.

j3nsch commented 2 years ago

Vielleicht könnte man mit charindex nach dem T in den Zeitstempel suchen und damit die Länge des substring steuern. Also "finde alle Anfaben mit 'T" und setze dann den Wert auf den Teilstring bis zu dem Zeichen davor".

j3nsch commented 2 years ago

@vgerlach Könntest Du das SQL dafür bauen bzw. prüfen ob das so funktionieren würde?

alw-bsz commented 2 years ago

Bei uns kommen die Datums-/Zeitwerte ebenfalls in den oben genannten Varianten vor. Mischformen, z.B. die Belegung von completed_date bei älteren Datensätze mit 2018-08-20, später dann mit 2019-02-11T00:00:00+01:00 gibt es bei uns auch. In anderen Instanzen geht das allerdings ziemlich durcheinander, d.h. ein Rückschluss auf das Update auf 4.7 ist kaum möglich. Oder es liegt an SWORD-Importen (dafür käme im Wesentlichen nur DeepGreen in Betracht).

j3nsch commented 2 years ago

Ja, mittlerweile denke ich das liegt (schon immer) am Dates Helper bzw. an der Date-Klasse, wenn sie mit Zend_Date initialisiert wird. Es gab nur keinen Test dafür, weil es nur im Zusammenspiel beider Klassen zu Problemen kommt. In den Unit Tests für Date, wird kein Zend_Date verwendet, und in den Tests für Dates wird nicht geprüft was in der Datenbank landet. Ob SWORD auch solche Angaben erzeugt ist eigentlich egal, weil Publish es auf jeden Fall macht. Ich werde das für die Zukunft fixen und ein Bereinigungsskript für das nächste Update schreiben.

j3nsch commented 2 years ago

Die Dates Klasse ist mindestens von 2011.

vgerlach commented 2 years ago

Klar, sollte kein Problem sein.


Von: Jens Schwidder @.***> Gesendet: Montag, 4. Juli 2022 11:02 An: OPUS4/framework Cc: Gerlach, Viktoria; Mention Betreff: Re: [OPUS4/framework] Datumsangaben nicht als Zeitstempel speichern (Issue #273)

@vgerlachhttps://github.com/vgerlach Könntest Du das SQL dafür bauen bzw. prüfen ob das so funktionieren würde?

— Reply to this email directly, view it on GitHubhttps://github.com/OPUS4/framework/issues/273#issuecomment-1173551910, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AD4UGIJCWZCDET6553X7JMLVSKSDDANCNFSM52NLEMIQ. You are receiving this because you were mentioned.Message ID: @.***>

j3nsch commented 2 years ago

Das folgende SQL, scheint zu funktionieren.

UPDATE documents SET server_date_modified = SUBSTRING_INDEX(server_date_modified, 'T', 1) WHERE server_date_modified LIKE '%T%';

Das muss dann für alle relevanten Spalten ausgeführt werden, um die String auf das Datum zu kürzen. Ich werde dafür eine Klasse im Framework schreiben, die dann mit einem Skript ausgeführt werden kann. Vielleicht sollte man gleich ein Kommando daraus machen. Das ganze kann dann für 4.7.2 in den Update-Prozess eingebunden werden, so dass die Bereinigung automatisch stattfindet. Das würde ich aber erst kurz vor dem Release aktivieren, um während der Entwicklung das Testen nicht zu verkomplizieren.