SORMAS-Project
SORMAS-Project copied to clipboard
#6700 replace joda time
Fixes #6700. Draft because I slowly replace joda-time
- [x] Decide how to deal with null values in
DateHelper.getStartOf
/getEndOf
-> Keepnull -> now()
behaviour for this ticket, follow-up ticket: #10042 - [ ] Clarify that formatting now depending on
I18nProperties.getUserLanguage()
is okay.- [ ] Github Actions fail because of not set
/etc/localtime
- [ ] DateHelperTest markers are dealt with (commented out test cases)
- [ ] Github Actions fail because of not set
- [x] Obsolete class DateFilter together with
DateHelper.findDateBounds
andDateHelper.findDatePrefix
are removed. - [ ] Fix breaking tests on Github Actions:
WARNING | /etc/localtime does not point to zoneinfo-compatible timezone name
- [ ] Before merging, update from
development
just in case someone added joda-time in the meantime.
I now found out that DateHelper.getStartOf/getEndOf
methods implicitly converted null
to LocalDateTime.now()
and now many backend tests fail. I'm uncertain if this behaviour is desirable or not.
@Test
public void testGetStartOfDay() {
// Previous joda-time implementation: null = now
Date date = null;
assertNull(new org.joda.time.LocalDateTime(date).withTime(0, 0, 0, 0).toDate());
// >>> java.lang.AssertionError: expected null, but was:<Tue Jul 26 00:00:00 CEST 2022>
}
Affected logic found by unit tests:
// CaseService
public void updateFollowUpDetails(Case caze, boolean followUpStatusChangedByUser) {
//...
// NPE happens here
if (DateHelper.getStartOfDay(currentFollowUpUntil).before(DateHelper.getStartOfDay(untilDate))) {
caze.setOverwriteFollowUpUntil(false);
}
// ContactService
public void updateFollowUpDetails(Contact contact, boolean followUpStatusChangedByUser) {
//...
// NPE happens here
if (DateHelper.getStartOfDay(currentFollowUpUntil).before(DateHelper.getStartOfDay(untilDate))) {
contact.setOverwriteFollowUpUntil(false);
}
// VisitService
public Predicate buildRelevantVisitsFilter(Person person, Disease disease, Date startDate, Date endDate, CriteriaBuilder cb, Root<Visit> from) {
// NPE happens here
startDate = DateHelper.getStartOfDay(startDate);
endDate = DateHelper.getEndOfDay(endDate);
Predicate filter = cb.and(cb.equal(from.get(Visit.PERSON), person), cb.equal(from.get(Visit.DISEASE), disease));
filter = CriteriaBuilderHelper.and(
cb,
filter,
cb.greaterThanOrEqualTo(from.get(Visit.VISIT_DATE_TIME), DateHelper.subtractDays(startDate, FollowUpLogic.ALLOWED_DATE_OFFSET)),
cb.lessThanOrEqualTo(from.get(Visit.VISIT_DATE_TIME), DateHelper.addDays(endDate, FollowUpLogic.ALLOWED_DATE_OFFSET)));
# CaseFacadeEjb.getIndexList: H2Function.timestamp_subtract_days fails with null Date
Caused by: java.lang.NullPointerException
at de.symeda.sormas.api.utils.DateHelper.subtractDays(DateHelper.java:549)
at de.symeda.sormas.backend.H2Function.timestamp_subtract_days(H2Function.java:49)
CaseFacadeEjb.getIndexList: StackTrace
javax.persistence.PersistenceException: org.hibernate.exception.GenericJDBCException: could not extract ResultSet
at org.hibernate.internal.ExceptionConverterImpl.convert(ExceptionConverterImpl.java:154)
at org.hibernate.query.internal.AbstractProducedQuery.list(AbstractProducedQuery.java:1626)
at org.hibernate.query.Query.getResultList(Query.java:165)
at org.hibernate.query.criteria.internal.compile.CriteriaQueryTypeQueryAdapter.getResultList(CriteriaQueryTypeQueryAdapter.java:76)
at de.symeda.sormas.backend.util.QueryHelper.getResultList(QueryHelper.java:141)
at de.symeda.sormas.backend.caze.CaseFacadeEjb.getIndexList(CaseFacadeEjb.java:596)
...
at org.jboss.weld.interceptor.proxy.InterceptorMethodHandler.invoke(InterceptorMethodHandler.java:41)
at org.jboss.weld.bean.proxy.CombinedInterceptorAndDecoratorStackMethodHandler.invoke(CombinedInterceptorAndDecoratorStackMethodHandler.java:53)
at de.symeda.sormas.backend.caze.CaseFacadeEjb$CaseFacadeEjbLocal$Proxy$_$$_WeldSubclass.getIndexList(Unknown Source)
at de.symeda.sormas.backend.caze.CaseFacadeEjb$CaseFacadeEjbLocal$Proxy$_$$_WeldClientProxy.getIndexList(Unknown Source)
at de.symeda.sormas.backend.caze.CaseFacadeEjbTest.testCaseCriteriaChangedSinceLastShareWithReportingTool(CaseFacadeEjbTest.java:2670)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
...
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:210)
Caused by: org.hibernate.exception.GenericJDBCException: could not extract ResultSet
at org.hibernate.exception.internal.StandardSQLExceptionConverter.convert(StandardSQLExceptionConverter.java:42)
at org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:113)
at org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:99)
at org.hibernate.engine.jdbc.internal.ResultSetReturnImpl.extract(ResultSetReturnImpl.java:67)
at org.hibernate.loader.Loader.getResultSet(Loader.java:2322)
...
at org.hibernate.query.internal.AbstractProducedQuery.doList(AbstractProducedQuery.java:1649)
at org.hibernate.query.internal.AbstractProducedQuery.list(AbstractProducedQuery.java:1617)
... 54 more
Caused by: org.h2.jdbc.JdbcSQLNonTransientException: Fehler beim Aufruf eine benutzerdefinierten Funktion: "timestamp_subtract_days(null, 14): null"
Exception calling user-defined function: "timestamp_subtract_days(null, 14): null"; SQL statement:
select distinct case0_.id as col_0_0_, case0_.uuid as col_1_0_, case0_.epidNumber as col_2_0_, case0_.externalID as col_3_0_, case0_.externalToken as col_4_0_, case0_.internalToken as col_5_0_, person1_.uuid as col_6_0_, person1_.firstName as col_7_0_, person1_.lastName as col_8_0_, case0_.disease as col_9_0_, case0_.diseaseVariant as col_10_0_, case0_.diseaseDetails as col_11_0_, case0_.caseClassification as col_12_0_, case0_.investigationStatus as col_13_0_, person1_.presentCondition as col_14_0_, case0_.reportDate as col_15_0_, case0_.creationDate as col_16_0_, region3_.uuid as col_17_0_, district4_.uuid as col_18_0_, facility5_.uuid as col_19_0_, facility5_.name as col_20_0_, case0_.healthFacilityDetails as col_21_0_, pointofent6_.uuid as col_22_0_, pointofent6_.name as col_23_0_, case0_.pointOfEntryDetails as col_24_0_, user7_.uuid as col_25_0_, case0_.outcome as col_26_0_, person1_.approximateAge as col_27_0_, person1_.approximateAgeType as col_28_0_, person1_.birthdate_dd as col_29_0_, person1_.birthdate_mm as col_30_0_, person1_.birthdate_yyyy as col_31_0_, person1_.sex as col_32_0_, case0_.quarantineTo as col_33_0_, case0_.completeness as col_34_0_, case0_.followUpStatus as col_35_0_, case0_.followUpUntil as col_36_0_, person1_.symptomJournalStatus as col_37_0_, case0_.vaccinationStatus as col_38_0_, case0_.changeDate as col_39_0_, facility5_.id as col_40_0_, region8_.uuid as col_41_0_, district9_.uuid as col_42_0_, district9_.name as col_43_0_, case when (case0_.reportingUser_id is not null) and case0_.reportingUser_id=27 or 1=1 then TRUE else FALSE end as col_44_0_, (select (select count(visits41_.caze_id) from Visit visits41_ where case38_.id = visits41_.caze_id) from cases case38_ where case38_.id=case0_.id) as col_45_0_, greatest(case0_.changeDate, person1_.changeDate) as col_46_0_ from cases case0_ left outer join Person person1_ on case0_.person_id=person1_.id left outer join Location location2_ on person1_.address_id=location2_.id left outer join Region region3_ on case0_.region_id=region3_.id left outer join District district4_ on case0_.district_id=district4_.id left outer join Facility facility5_ on case0_.healthFacility_id=facility5_.id left outer join PointOfEntry pointofent6_ on case0_.pointOfEntry_id=pointofent6_.id left outer join users user7_ on case0_.surveillanceOfficer_id=user7_.id left outer join Region region8_ on case0_.responsibleRegion_id=region8_.id left outer join District district9_ on case0_.responsibleDistrict_id=district9_.id left outer join users user10_ on case0_.reportingUser_id=user10_.id left outer join Hospitalization hospitaliz11_ on case0_.hospitalization_id=hospitaliz11_.id left outer join previoushospitalization previousho12_ on hospitaliz11_.id=previousho12_.hospitalization_id left outer join ClinicalCourse clinicalco13_ on case0_.clinicalCourse_id=clinicalco13_.id left outer join Symptoms symptoms14_ on case0_.symptoms_id=symptoms14_.id left outer join Therapy therapy15_ on case0_.therapy_id=therapy15_.id left outer join HealthConditions healthcond16_ on case0_.healthConditions_id=healthcond16_.id left outer join MaternalHistory maternalhi17_ on case0_.maternalHistory_id=maternalhi17_.id left outer join PortHealthInfo porthealth18_ on case0_.portHealthInfo_id=porthealth18_.id left outer join sormastosormasorigininfo sormastoso19_ on case0_.sormasToSormasOriginInfo_id=sormastoso19_.id left outer join sormastosormasshareinfo sormastoso20_ on case0_.id=sormastoso20_.caze_id left outer join EpiData epidata21_ on case0_.epiData_id=epidata21_.id left outer join exposures exposures22_ on epidata21_.id=exposures22_.epiData_id left outer join Location location23_ on exposures22_.location_id=location23_.id left outer join activityascase activities24_ on epidata21_.id=activities24_.epiData_id left outer join Location location25_ on activities24_.location_id=location25_.id left outer join samples samples26_ on case0_.id=samples26_.associatedCase_id left outer join PathogenTest pathogente27_ on samples26_.id=pathogente27_.sample_id left outer join Person person28_ on case0_.person_id=person28_.id left outer join Location location29_ on person28_.address_id=location29_.id left outer join Visit visits30_ on case0_.id=visits30_.caze_id left outer join surveillancereports surveillan31_ on case0_.id=surveillan31_.caze_id left outer join Person person32_ on case0_.person_id=person32_.id left outer join immunization immunizati33_ on person32_.id=immunizati33_.person_id left outer join Vaccination vaccinatio34_ on immunizati33_.id=vaccinatio34_.immunization_id and ((select symptoms35_.onsetDate from Symptoms symptoms35_ where symptoms35_.id=case0_.symptoms_id)>at_end_of_day(case when vaccinatio34_.vaccinationDate is null then timestamp_subtract_days(vaccinatio34_.reportDate,14) else vaccinatio34_.vaccinationDate end) or ((select symptoms36_.onsetDate from Symptoms symptoms36_ where symptoms36_.id=case0_.symptoms_id) is null) and case0_.reportDate>at_end_of_day(case when vaccinatio34_.vaccinationDate is null then timestamp_subtract_days(vaccinatio34_.reportDate,14) else vaccinatio34_.vaccinationDate end)) left outer join immunization immunizati37_ on vaccinatio34_.immunization_id=immunizati37_.id where case0_.deleted=? and (exists (select case40_.id from externalshareinfo externalsh39_ left outer join cases case40_ on externalsh39_.caze_id=case40_.id where case40_.id=case0_.id group by case40_.id having case0_.changeDate>max(externalsh39_.creationDate) and (case0_.changeDate is not null) or symptoms14_.changeDate>max(externalsh39_.creationDate) and (symptoms14_.changeDate is not null) or hospitaliz11_.changeDate>max(externalsh39_.creationDate) and (hospitaliz11_.changeDate is not null) or previousho12_.changeDate>max(externalsh39_.creationDate) and (previousho12_.changeDate is not null) or therapy15_.changeDate>max(externalsh39_.creationDate) and (therapy15_.changeDate is not null) or clinicalco13_.changeDate>max(externalsh39_.creationDate) and (clinicalco13_.changeDate is not null) or healthcond16_.changeDate>max(externalsh39_.creationDate) and (healthcond16_.changeDate is not null) or maternalhi17_.changeDate>max(externalsh39_.creationDate) and (maternalhi17_.changeDate is not null) or porthealth18_.changeDate>max(externalsh39_.creationDate) and (porthealth18_.changeDate is not null) or sormastoso19_.changeDate>max(externalsh39_.creationDate) and (sormastoso19_.changeDate is not null) or sormastoso20_.changeDate>max(externalsh39_.creationDate) and (sormastoso20_.changeDate is not null) or epidata21_.changeDate>max(externalsh39_.creationDate) and (epidata21_.changeDate is not null) or exposures22_.changeDate>max(externalsh39_.creationDate) and (exposures22_.changeDate is not null) or location23_.changeDate>max(externalsh39_.creationDate) and (location23_.changeDate is not null) or activities24_.changeDate>max(externalsh39_.creationDate) and (activities24_.changeDate is not null) or location25_.changeDate>max(externalsh39_.creationDate) and (location25_.changeDate is not null) or samples26_.changeDate>max(externalsh39_.creationDate) and (samples26_.changeDate is not null) or pathogente27_.changeDate>max(externalsh39_.creationDate) and (pathogente27_.changeDate is not null) or person28_.changeDate>max(externalsh39_.creationDate) and (person28_.changeDate is not null) or location29_.changeDate>max(externalsh39_.creationDate) and (location29_.changeDate is not null) or visits30_.changeDate>max(externalsh39_.creationDate) and (visits30_.changeDate is not null) or surveillan31_.changeDate>max(externalsh39_.creationDate) and (surveillan31_.changeDate is not null) or vaccinatio34_.changeDate>max(externalsh39_.creationDate) and (vaccinatio34_.changeDate is not null) or immunizati37_.changeDate>max(externalsh39_.creationDate) and (immunizati37_.changeDate is not null))) and case0_.dontShareWithReportingTool=FALSE order by greatest(case0_.changeDate, person1_.changeDate) desc limit ? [90105-210]
at org.h2.message.DbException.getJdbcSQLException(DbException.java:573)
at org.h2.message.DbException.getJdbcSQLException(DbException.java:496)
at org.h2.message.DbException.get(DbException.java:216)
at org.h2.message.DbException.convertInvocation(DbException.java:443)
at org.h2.schema.FunctionAlias$JavaMethod.execute(FunctionAlias.java:508)
at org.h2.schema.FunctionAlias$JavaMethod.getValue(FunctionAlias.java:345)
at org.h2.expression.function.JavaFunction.getValue(JavaFunction.java:40)
at org.h2.expression.SearchedCase.getValue(SearchedCase.java:31)
at org.h2.schema.FunctionAlias$JavaMethod.execute(FunctionAlias.java:453)
at org.h2.schema.FunctionAlias$JavaMethod.getValue(FunctionAlias.java:345)
at org.h2.expression.function.JavaFunction.getValue(JavaFunction.java:40)
at org.h2.expression.condition.Comparison.getValue(Comparison.java:222)
at org.h2.expression.condition.ConditionAndOr.getValue(ConditionAndOr.java:106)
at org.h2.expression.condition.ConditionAndOr.getValue(ConditionAndOr.java:110)
at org.h2.expression.Expression.getBooleanValue(Expression.java:332)
at org.h2.table.TableFilter.isOk(TableFilter.java:505)
at org.h2.table.TableFilter.next(TableFilter.java:454)
at org.h2.table.TableFilter.next(TableFilter.java:464)
...
at org.h2.table.TableFilter.next(TableFilter.java:464)
at org.h2.command.query.Select$LazyResultQueryFlat.fetchNextRow(Select.java:1825)
at org.h2.result.LazyResult.hasNext(LazyResult.java:78)
at org.h2.result.FetchedResult.next(FetchedResult.java:34)
at org.h2.command.query.Select.queryFlat(Select.java:728)
at org.h2.command.query.Select.queryWithoutCache(Select.java:833)
at org.h2.command.query.Query.queryWithoutCacheLazyCheck(Query.java:197)
at org.h2.command.query.Query.query(Query.java:494)
at org.h2.command.query.Query.query(Query.java:457)
at org.h2.command.CommandContainer.query(CommandContainer.java:256)
at org.h2.command.Command.executeQuery(Command.java:190)
at org.h2.jdbc.JdbcPreparedStatement.executeQuery(JdbcPreparedStatement.java:128)
at org.hibernate.engine.jdbc.internal.ResultSetReturnImpl.extract(ResultSetReturnImpl.java:57)
... 69 more
Caused by: java.lang.NullPointerException
at de.symeda.sormas.api.utils.DateHelper.subtractDays(DateHelper.java:549)
at de.symeda.sormas.backend.H2Function.timestamp_subtract_days(H2Function.java:49)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.h2.schema.FunctionAlias$JavaMethod.execute(FunctionAlias.java:495)
... 127 more
Other findings to be discussed:
- Obsolete?
de.symeda.sormas.ui.utils.DateFilter
- DateHelper.findDateBounds
- DateHelper.findDatePrefix
- Relevance?
de.symeda.sormas.ui.EditFormLayoutCharting
I think having it return null instead of new is favorable, so we will have to add explicit checks to those failing usages, which will cause some work. I tend to think that methods like getStartOfDay should also be possible to handle null (and return null). Let's discuss.
1. Obsolete? `de.symeda.sormas.ui.utils.DateFilter` * DateHelper.findDateBounds * DateHelper.findDatePrefix
Opinion by @MartinWahnschaffe: Remove not used code.
2. Relevance? `de.symeda.sormas.ui.EditFormLayoutCharting`
Opinion by @MartinWahnschaffe: Keep class, remove joda-time usage.
I think having it return null instead of new is favorable, so we will have to add explicit checks to those failing usages, which will cause some work. I tend to think that methods like getStartOfDay should also be possible to handle null (and return null). Let's discuss.
Agreed with @MartinWahnschaffe that we change the framework now but don't alter the behaviour but will move that to a follow-up ticket.
@MateStrysewske I have reviewed the non-Android parts of this PR. Would be great if you could review the Android part, since I'm not supposed to review my own code :-D
Marked as draft to make sure merge conflicts are taken care off properly and that the change history will be kept when merging.
@StefanKock Findings have been addressed and dev was merged once again. After CI is done, this can be merged from my point of view.