Onto-Med / top-backend

Spring Boot based backend of the TOP Framework
MIT License
0 stars 1 forks source link

Where to get expressionFunctions from #208

Closed fmatthies closed 6 months ago

fmatthies commented 6 months ago

Right now

  @Override
  @Cacheable("expressionFunctions")
  public ResponseEntity<List<ExpressionFunction>> getExpressionFunctions(String type) {
    return new ResponseEntity<>(
        Stream.concat(
                c2r.getFunctions().stream().map(FunctionEntity::getFunction),
                Arrays.stream(SONG.getExpressionFunctions()).map(f -> f.type("textFunction")))
            .filter(f -> StringUtils.isBlank(type) || type.equals(f.getType()))
            .sorted(
                Comparator.comparing(ExpressionFunction::getType)
                    .thenComparing(ExpressionFunction::getTitle))
            .collect(Collectors.toList()),
        HttpStatus.OK);
  }

the general SONG class is used to get them for the document search. I don't have a scenario at hand, but one could think of a text query language that has additional or fewer functions. Wouldn't it be better to delegate the getting to the actual adapter implementation (e.g. the ElasticsearchSong)?

ChristophB commented 6 months ago

This is just my two cents on the subject:

This means that the available functions for creating a model are restricted to specific document source types (e.g., Elasticsearch). I think this contradicts with the idea of having generic models that can be used to query document sources of any type. It also becomes cumbersome to recreate the same model for another data source, such as Meilisearch, just because one fancy function of Elasticsearch is not supported.

The purpose of SON should be to provide a generic framework for searching documents. Adapters need to implement all functions of that framework and if one function is not supported, an exception should be thrown. Something like: "This data source does not support the function 'AND' you have used in your query."

fmatthies commented 6 months ago

You're totally right. I was focussed on the micro and didn't see the macro level...

"This data source does not support the function 'AND' you have used in your query."

But better yet, the adapter needs to implement the function somehow with its own capabilities and maybe just throw a warning that the function in question might not perform very well or some such

fmatthies commented 6 months ago

Moved this to top-document-query