adessoSE/budgeteer

Risk of calling WorkRecordRepository.updateDailyRates with null values

FrederikSchlemmer opened this issue · 3 comments

When updating the daily rates in the PersonService by calling the updateDailyRates method in the WorkRecordRepository there is the risk of executing this query with null values.

This code snippet show the issue:
workRecordRepository.updateDailyRates(rate.getBudget().getId(), person.getPersonId(), rate.getDateRange().getStartDate(), rate.getDateRange().getEndDate(), rate.getRate());

The rate is an instance of the class PersonRate, which got a NoArgsConstructor and a reset method. That's why there is the possibility of null values.
Additionally, it should be investigated if the reset method should remain. By resetting a PersonRate there is the chance of inconsistent data because resetting the rate loses the connection to the WorkRecords. Then the rate remains in the work records but the rate itself is reset.

@tinne I would be happy about a review on your part. I have added some more thoughts and problems.

tinne commented

Does not make sense to me either.

Nor do the equals/hashCode pair which in fact overwrite Lomboks @EqualsAndHashCode implied in @Data.

Und dann brauchen wir noch vorher so etwas wie

update WorkRecordEntity r s
et r.dailyRate = null 
where r.editedManually = false and r.person.id=:personId and r.budget.id is null

oder besser zu irgendeinem Zeitpunkt (ungetestet)

update WorkRecordEntity r set r.dailyRate = null 
where r.editedManually = false and r.person.id=:personId and r.budget.id is null and not exists ( 
    select PersonRate pr 
    where pr.budget.id=:budgetId and pr.person.id=:personId 
    and r.date between pr.fromDate and pr.toDate
)

, falls Ihr letzteres in HQL hinbekommt oder das in SQL, Predicates oder wieauchimmer abbilden wollt.

Does not make sense to me either.

Nor do the equals/hashCode pair which in fact overwrite Lomboks @EqualsAndHashCode implied in @Data.

Und dann brauchen wir noch vorher so etwas wie

update WorkRecordEntity r s
et r.dailyRate = null 
where r.editedManually = false and r.person.id=:personId and r.budget.id is null

oder besser zu irgendeinem Zeitpunkt (ungetestet)

update WorkRecordEntity r set r.dailyRate = null 
where r.editedManually = false and r.person.id=:personId and r.budget.id is null and not exists ( 
    select PersonRate pr 
    where pr.budget.id=:budgetId and pr.person.id=:personId 
    and r.date between pr.fromDate and pr.toDate
)

, falls Ihr letzteres in HQL hinbekommt oder das in SQL, Predicates oder wieauchimmer abbilden wollt.

There is no need to look in the database for DailyRates that are stored with a Null value in the Budget ID or Person ID. The IDs are longs, which cannot take a null value.

It was also considered that if errors occur during the update, this data will not be included in the PersonEntity as DailyRate.