Project

General

Profile

bug #9814

Advanced search webservice for area throws NPE (mexico portal)

Added by Andreas Müller about 1 month ago. Updated about 1 month ago.

Status:
Closed
Priority:
New
Category:
cdmlib
Target version:
Start date:
10/18/2021
Due date:
% Done:

100%

Severity:
critical
Found in Version:
Tags:

Description

... Die Area-Terme in der Suche fehlen gerade komplett, das der Webservice nichts zurückliefert: https://132.248.13.83/cdmserver/cdm_flora_mexico/description/namedAreasInUse.json?includeAllParents=true&pageIndex=0&pageSize=500 :

... 98 common frames omitted Caused by: java.lang.NullPointerException: null at eu.etaxonomy.cdm.persistence.dto.TermDto.getVocRepresentation_L10n_abbreviatedLabel(TermDto.java:154) ... 108 common frames omitted  

Das ist offensichtlich ein Bug im TermDto, bis dieser behoben ist sollten wir darauf achten, dass für alle Area-Terme das abbreviatedLabel angegeben ist.

Associated revisions

Revision d4542f66 (diff)
Added by Andreas Müller about 1 month ago

ref #9814 fix NPE in TermDto.getVocRepresentation_L10n (+..._abbreviatedLabel)

Revision dfd8da23 (diff)
Added by Katja Luther about 1 month ago

ref #9814: add vocDto to termDto

Revision 4af8adcf (diff)
Added by Katja Luther about 1 month ago

ref #9814: rename findByTitleasDto method to findByTitleAsDtoWithVocDto

Revision 13866815 (diff)
Added by Katja Luther about 1 month ago

ref #9814: adapt method in editor

Revision f1f5966b (diff)
Added by Andreas Müller about 1 month ago

ref #9814 cache vocabularyDtos when loading termDTOs

Revision cb291ebd (diff)
Added by Andreas Müller about 1 month ago

ref #9814 fix vocabularyDtos caching

History

#1 Updated by Andreas Müller about 1 month ago

  • Description updated (diff)

This is strange, the actual method getVocRepresentation_L10n_abbreviatedLabel() is never called in cdmlib so I don't understand why an NPE can be thrown.

#2 Updated by Andreas Müller about 1 month ago

  • Status changed from New to Feedback
  • Assignee changed from Andreas Müller to Andreas Kohlbecker
  • % Done changed from 0 to 30

I fixed the NPE so it does not throw an exception anymore. However, we need to discuss how the TermDTO should behave. Currently the constructor only takes the vocabulary.uuid and therefor no representations for vocabulary are available. Do we need those?

As said before the method is never called directly in cdmlib. Only jason seems to call it during serealization. If none of the webservice users(probably only the dataportal) needs the label we should mybe remove this method.

This issue came with commit cdmlib:71559ffe9c (KL, 2021-09-15) which removed vocRepresentations from the termDTO.

KL + AK can you please both check if you think the vocabulary representation is always needed in this DTO or only if it is explicitly set via setVocabularyDto(TermVocabularyDto vocabularyDto) method or by using the factory method fromTerm(...)?

I give this ticket first to AK for check if the method is needed at all. If yes, we should have some tests that guarantee that voc labels are available in the context where it is needed.

#3 Updated by Andreas Kohlbecker about 1 month ago

  • Assignee changed from Andreas Kohlbecker to Katja Luther

this method is only used in the TaxEditor:

eu.etaxonomy.taxeditor.store/src/main/java/eu/etaxonomy/taxeditor/view/search/facet/term/TermSearchResult.java:            
facets.add(new Facet(((TermDto)content).getVocRepresentation_L10n(), termType!=null?termType.getLabel():null));

thus passing the ticket to you Katja

#4 Updated by Andreas Müller about 1 month ago

I checked advance search on test servers and it works again: http://test.e-taxonomy.eu/dataportal/preview/euromed/cdm_dataportal/search

#5 Updated by Katja Luther about 1 month ago

  • Status changed from Feedback to Resolved
  • Assignee changed from Katja Luther to Andreas Kohlbecker

Added the vocDto to termDto when creating the termDto because the voc representations are needed for the term search. please review, if everything else still works. Already tested the advanced search for e+m

#6 Updated by Andreas Kohlbecker about 1 month ago

  • Status changed from Resolved to Feedback
  • Assignee changed from Andreas Kohlbecker to Katja Luther
  1. I think adding the VocDto to the termDTOs bloats the reponses with a lot of redundant information. In most cases the terms returned by /description/namedAreasInUse.json will belong to the same vocabulary. Adding the vocabulary uuids to the termDTO would be much more efficient. If needed, we could introduce a TermListDTO which allows for a normalized representation of termDTOs and VocabDTOs.
  2. In the reponse of http://test.e-taxonomy.eu/cdmserver/euromed/description/namedAreasInUse.json the vocabularyDto is always null, so something is not working as expected here, otherwise this it not really relevant if we agree in not adding the vocDTo to the termDTOs

#7 Updated by Andreas Müller about 1 month ago

  • Target version changed from Release 5.29 to Release 5.28

#8 Updated by Katja Luther about 1 month ago

  • Target version changed from Release 5.28 to Release 5.29

Andreas Kohlbecker wrote:

  1. I think adding the VocDto to the termDTOs bloats the reponses with a lot of redundant information. In most cases the terms returned by /description/namedAreasInUse.json will belong to the same vocabulary. Adding the vocabulary uuids to the termDTO would be much more efficient. If needed, we could introduce a TermListDTO which allows for a normalized representation of termDTOs and VocabDTOs.
  2. In the reponse of http://test.e-taxonomy.eu/cdmserver/euromed/description/namedAreasInUse.json the vocabularyDto is always null, so something is not working as expected here, otherwise this it not really relevant if we agree in not adding the vocDTo to the termDTOs

The vocabulary Dto is added in the dao method, so I changed the name of this method to findByTitleAsDtoWithVocDto, actually it is used only for the term search, where the vocabulary information is needed.

#9 Updated by Andreas Müller about 1 month ago

  • Target version changed from Release 5.29 to Release 5.28

#10 Updated by Andreas Müller about 1 month ago

who is now in charge of this ticket?

#11 Updated by Andreas Müller about 1 month ago

In the mentioned method you iterate over all returned dtos and for each you run a query to retrieve the VocDto. This is highly inperformant. As the vocabulary is usually highly redundant (most terms come from the same voc). Why don't you hold the result in a Map and only run the query for those vocabularies not yet loaded?

#12 Updated by Andreas Müller about 1 month ago

Andreas Müller wrote:

In the mentioned method you iterate over all returned dtos and for each you run a query to retrieve the VocDto. This is highly inperformant. As the vocabulary is usually highly redundant (most terms come from the same voc). Why don't you hold the result in a Map and only run the query for those vocabularies not yet loaded?

I fixed this. Please review.

#13 Updated by Andreas Müller about 1 month ago

  • Status changed from Feedback to Resolved
  • % Done changed from 30 to 50

I think in general this ticket is fixed as the webservice and TaxEditor do use 2 different methods to initialize the TermDTO (with or without full vocabulary DTO). So I suggest to close it once KL has reviewed my last changes.

However, it shows the DTO handling is still very chaotic and urgently needs more structure. But this needs to be done in another context I guess.

#14 Updated by Katja Luther about 1 month ago

  • Status changed from Resolved to Closed
  • Assignee changed from Katja Luther to Andreas Müller

This works as expected.

#15 Updated by Katja Luther about 1 month ago

  • % Done changed from 50 to 100

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 40 MB)