Project

General

Profile

task #8437

ITermTreeService provides a list method which supports filtering by TermTree.termType,

Added by Andreas Kohlbecker about 2 months ago. Updated 25 days ago.

Status:
Resolved
Priority:
Priority14
Category:
cdmlib
Target version:
Start date:
08/01/2019
Due date:
% Done:

80%

Severity:
normal

Description

e.g.:


public List<TermTree> list(TermType termType, Integer limit, Integer start, List<OrderHint> orderHints, List<String> propertyPaths);


public Pager<TermTree> page(TermType termType, Integer pageSize, Integer pageIndex, List<OrderHint> orderHints, List<String> propertyPaths);

Related issues

Precedes Edit - task #8436: featureTree/* services should filter the termTrees by the TermType Feature Closed

Associated revisions

Revision b820a890 (diff)
Added by Andreas Müller about 2 months ago

ref #8437 first implementation for listBy termType for TermTreeService, not yet tested

Revision 2ef4c225 (diff)
Added by Andreas Kohlbecker about 2 months ago

ref #8437 using the listByRestriction method in favour of a new untested dao method & implementing page method in ITermTreeService

Revision 19b30c4f (diff)
Added by Andreas Kohlbecker about 1 month ago

ref #8437 ITermTreeService termType Feature as filter includes also Character

Revision 1a5b2194 (diff)
Added by Andreas Kohlbecker about 1 month ago

ref #8437 fixing LIE in DerivedUnitFacadeController

Revision acdfa184 (diff)
Added by Andreas Kohlbecker 25 days ago

ref #8437 using TermType.getGeneralizationOf() to expand the termType restriction

History

#1 Updated by Andreas Kohlbecker about 2 months ago

  • Precedes task #8436: featureTree/* services should filter the termTrees by the TermType Feature added

#2 Updated by Andreas Müller about 2 months ago

  • Status changed from New to In Progress
  • Priority changed from New to Priority14
  • Target version changed from Unassigned CDM tickets to Release 5.9
  • % Done changed from 0 to 40

implemented but no tests yet, but maybe you can integrate into controller already

#3 Updated by Andreas Kohlbecker about 2 months ago

  • Status changed from In Progress to Resolved
  • % Done changed from 40 to 50

please can you review my changes?

#4 Updated by Andreas Müller about 2 months ago

  • Status changed from Resolved to Feedback
  • Assignee changed from Andreas Müller to Andreas Kohlbecker

Have you tested with termType = Character which is a subtype for Feature. I don't think the "Restrictions" implementation will manage this correctly.

#5 Updated by Andreas Müller about 2 months ago

So we definetely should write tests before closing.

#6 Updated by Andreas Kohlbecker about 1 month ago

  • Assignee changed from Andreas Kohlbecker to Andreas Müller

Andreas Müller wrote:

Have you tested with termType = Character which is a subtype for Feature. I don't think the "Restrictions" implementation will manage this correctly.

I did not understand this as requirement and I see two problems whith this:

  1. From the TermTypes enum which is being used as filter it is not clear that using Feature will also include Character.
  2. How would it be possible to get TermTrees for Feature without Character ?

Therefore I suggest to allow passing a Set of TermTypes as filter:

public List<TermTree> list(Set<TermType> termType, Integer limit, Integer start, List<OrderHint> orderHints, List<String> propertyPaths);


public Pager<TermTree> page(Set<TermType> termType, Integer pageSize, Integer pageIndex, List<OrderHint> orderHints, List<String> propertyPaths);

which is supported by the restrictions api out of the box.

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

  • Assignee changed from Andreas Müller to Andreas Kohlbecker

Andreas Kohlbecker wrote:

Andreas Müller wrote:

Have you tested with termType = Character which is a subtype for Feature. I don't think the "Restrictions" implementation will manage this correctly.

I did not understand this as requirement and I see two problems whith this:

  1. From the TermTypes enum which is being used as filter it is not clear that using Feature will also include Character.
  2. How would it be possible to get TermTrees for Feature without Character ?

Therefore I suggest to allow passing a Set of TermTypes as filter:

public List<TermTree> list(Set<TermType> termType, Integer limit, Integer start, List<OrderHint> orderHints, List<String> propertyPaths);


public Pager<TermTree> page(Set<TermType> termType, Integer pageSize, Integer pageIndex, List<OrderHint> orderHints, List<String> propertyPaths);

which is supported by the restrictions api out of the box.

The javadoc says "Is subtype of {@link #Feature}" which to me is clear. Also the idea of the term types is to replace classes as classes are difficult to handle sometimes. But the semantics should be the same. Therefore if using list(class, ....) you also do not expect that you need to pass all the subclasses as a collection of classes. This is exactely the same behavior as for classes and it is by intention. Therefore I do not agree with the above solution.

#8 Updated by Andreas Kohlbecker about 1 month ago

  • Status changed from Feedback to Resolved
  • Assignee changed from Andreas Kohlbecker to Andreas Müller

Agreed ... and implemented in the way you pointed out.

#9 Updated by Andreas Müller 25 days ago

To me it looks like commit 1a5b2194 is not related to this ticket. Can you please check?

#10 Updated by Andreas Müller 25 days ago

  • Status changed from Resolved to Feedback
  • Assignee changed from Andreas Müller to Andreas Kohlbecker
  • % Done changed from 50 to 70

Andreas Kohlbecker wrote:

Agreed ... and implemented in the way you pointed out.

The current implementation is not generic as my first implementation was. It handles Characters as an explicit case. My first implementation did handle ALL sub-types. Currently only this 1 subtype exists, but in future there might be more.

Also it might be nice to have a little test that it works. Have you tested in some environment?

#11 Updated by Andreas Kohlbecker 25 days ago

  • Status changed from Feedback to Resolved
  • Assignee changed from Andreas Kohlbecker to Andreas Müller
  • % Done changed from 70 to 80

my implementation now is generic.

Also it might be nice to have a little test that it works. Have you tested in some environment?

I positively tested manually with the cdm_additivity_ontology database. Unit-tests do not yet exist.

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 40 MB)