Project

General

Profile

Actions

bug #9103

closed

missing exception handling in TaxoNodeDto

Added by Andreas Kohlbecker over 2 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Highest
Category:
cdmlib
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
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.

Actions #1

Updated by Andreas Kohlbecker over 2 years ago

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

Actions #2

Updated by Andreas Kohlbecker over 2 years ago

  • Status changed from New to Feedback
Actions #3

Updated by Katja Luther over 2 years ago

  • Assignee changed from Katja Luther to Andreas Kohlbecker

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

Actions #4

Updated by Andreas Kohlbecker over 2 years 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.

Actions #5

Updated by Andreas Müller over 2 years ago

  • Target version changed from Release 5.18 to Release 5.17
Actions #6

Updated by Andreas Kohlbecker over 2 years ago

  • % Done changed from 0 to 50
Actions #7

Updated by Andreas Kohlbecker over 2 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

Actions #8

Updated by Katja Luther almost 2 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.

Actions #9

Updated by Andreas Kohlbecker almost 2 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?

Actions #10

Updated by Katja Luther almost 2 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 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

Actions #11

Updated by Andreas Kohlbecker almost 2 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.

Actions

Also available in: Atom PDF