bug #9103
closedmissing exception handling in TaxoNodeDto
100%
Description
see
try{
TaxonNode parent = taxonNode.getParent();
parentUUID = parent == null? null:parent.getUuid();
}catch(Exception e){
parentUUID = null;
}
sortIndex = taxonNode.getSortIndex();
try{
classificationUUID = taxonNode.getClassification().getUuid();
}catch(Exception e){
classificationUUID = null;
}
LazyInitializationException are caught here, which can lean to missing data due to improper initialization.
Updated by Andreas Kohlbecker over 3 years ago
@Katja: has far as I could see, this stems from you, was there any urgent reason for this?
Updated by Andreas Kohlbecker over 3 years ago
- Status changed from New to Feedback
Updated by Katja Luther about 3 years ago
- Assignee changed from Katja Luther to Andreas Kohlbecker
sorry, I don't know anymore why I did this.
Updated by Andreas Kohlbecker about 3 years ago
- Status changed from Feedback to Resolved
Ok, so we should fix this right after the release by a proper implementation which
- checks for
taxonNode.getClassification() == nulland avoids catching exception - remove the try .. catch in case of
taxonNode.getParent()completely
This should be done quickly after the release to give problems time to pop up!
I'll keep this in the 5.17 milestone as resolved so that it can be done during the milestone cleanup.
Updated by Andreas Müller about 3 years ago
- Target version changed from Release 5.18 to Release 5.17
Updated by Andreas Kohlbecker about 3 years ago
- % Done changed from 0 to 50
Applied in changeset cdmlib|5d89c86976dd154266a6eae8c44b3874a2212e2a.
Updated by Andreas Kohlbecker about 3 years ago
- Assignee changed from Andreas Kohlbecker to Katja Luther
- Priority changed from New to Highest
- Target version changed from Release 5.17 to Release 5.18
done, please review
Updated by Katja Luther almost 3 years ago
- Status changed from Resolved to Feedback
- Assignee changed from Katja Luther to Andreas Kohlbecker
the null check for the classification is missing, the rest seems to be correct.
Updated by Andreas Kohlbecker almost 3 years ago
- Assignee changed from Andreas Kohlbecker to Katja Luther
Katja Luther wrote:
the null check for the classification is missing, the rest seems to be correct.
Good point, there was in deed a missing null check. Not exactly in the code that was affected by my changes, but a couple of lines above. I fixed this. But this part of the code seems to miss one case. The ITaxonTreeNode taxonTreeNode passed to the constructor can be a Classification. In this case the tileCache is not set at all. In this case it should be set to the title cache of the classification. Do you agree?
Updated by Katja Luther almost 3 years ago
Andreas Kohlbecker wrote:
Katja Luther wrote:
the null check for the classification is missing, the rest seems to be correct.
Good point, there was in deed a missing null check. Not exactly in the code that was affected by my changes, but a couple of lines above. I fixed this. But this part of the code seems to miss one case. The
ITaxonTreeNode taxonTreeNodepassed to the constructor can be aClassification. In this case thetileCacheis not set at all. In this case it should be set to the title cache of the classification. Do you agree?
Yes you are right this is not covered. I added the missing lines
Updated by Andreas Kohlbecker almost 3 years ago
- Status changed from Feedback to Closed
- Assignee changed from Katja Luther to Andreas Kohlbecker
- % Done changed from 50 to 100
the last commit fixes the potential NPE that was in the code after the previous commit. The code looks ok now and the ticket can be closed.