bug #9103
closed
missing exception handling in TaxoNodeDto
Added by Andreas Kohlbecker almost 4 years ago.
Updated over 3 years ago.
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.
@Katja: has far as I could see, this stems from you, was there any urgent reason for this?
- Status changed from New to Feedback
- Assignee changed from Katja Luther to Andreas Kohlbecker
sorry, I don't know anymore why I did this.
- 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() == null and 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.
- Target version changed from Release 5.18 to Release 5.17
- % Done changed from 0 to 50
- 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
- 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.
- 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?
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 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?
Yes you are right this is not covered. I added the missing lines
- 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.
Also available in: Atom
PDF