Project

General

Profile

bug #9103

missing exception handling in TaxoNodeDto

Added by Andreas Kohlbecker 11 months ago. Updated 6 months ago.

Status:
Closed
Priority:
Highest
Category:
cdmlib
Target version:
Start date:
06/26/2020
Due date:
% Done:

100%

Severity:
normal
Found in Version:

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.

Associated revisions

Revision 5d89c869 (diff)
Added by Andreas Kohlbecker 9 months ago

fix #9103 letting LIEs bubble up

Revision b68da265 (diff)
Added by Andreas Kohlbecker 8 months ago

ref #9103 adding missing init strategy element to TaxonController

Revision 94959be3 (diff)
Added by Andreas Kohlbecker 6 months ago

ref #9103 adding missing null check

Revision 6706afc3 (diff)
Added by Katja Luther 6 months ago

ref #9103: add handling of classifications in TaxonnodeDto

Revision da9e29c4 (diff)
Added by Katja Luther 6 months ago

ref #9103: add handling of classifications in TaxonnodeDto - continue

History

#1 Updated by Andreas Kohlbecker 11 months ago

@Katja: has far as I could see, this stems from you, was there any urgent reason for this?

#2 Updated by Andreas Kohlbecker 11 months ago

  • Status changed from New to Feedback

#3 Updated by Katja Luther 9 months ago

  • Assignee changed from Katja Luther to Andreas Kohlbecker

sorry, I don't know anymore why I did this.

#4 Updated by Andreas Kohlbecker 9 months ago

  • Status changed from Feedback to Resolved

Ok, so we should fix this right after the release by a proper implementation which

  1. checks for taxonNode.getClassification() == null and avoids catching exception
  2. 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.

#5 Updated by Andreas Müller 9 months ago

  • Target version changed from Release 5.18 to Release 5.17

#6 Updated by Andreas Kohlbecker 9 months ago

  • % Done changed from 0 to 50

#7 Updated by Andreas Kohlbecker 9 months 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

#8 Updated by Katja Luther 6 months 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.

#9 Updated by Andreas Kohlbecker 6 months 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?

#10 Updated by Katja Luther 6 months 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 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

#11 Updated by Andreas Kohlbecker 6 months 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.

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 40 MB)