From 1330b502c9833c0d4969a4a10337088463e447a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kalle=20W=C3=A5hlin?= Date: Mon, 23 Mar 2026 14:28:35 +0100 Subject: [PATCH 01/10] Adjust search to align with remodelled work vs instance category terms --- .../groovy/whelk/search2/Disambiguate.java | 22 ++-- .../src/main/groovy/whelk/search2/Query.java | 30 ++--- .../groovy/whelk/search2/SelectedFacets.java | 5 +- .../groovy/whelk/search2/SuggestQuery.java | 4 +- .../groovy/whelk/search2/VocabMappings.java | 28 +++-- .../whelk/search2/querytree/Condition.java | 9 +- .../whelk/search2/querytree/Property.java | 37 +++--- .../main/groovy/whelk/util/Restrictions.java | 26 +---- .../groovy/whelk/search2/QuerySpec.groovy | 110 ++++++++++++++++-- .../search2/querytree/EsQueryTreeSpec.groovy | 22 ++-- .../search2/querytree/QueryTreeSpec.groovy | 13 ++- .../whelk/search2/querytree/TestData.groovy | 60 +++++----- 12 files changed, 219 insertions(+), 147 deletions(-) diff --git a/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java b/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java index 7b60839ed4..1f64c23bf2 100644 --- a/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java +++ b/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java @@ -3,7 +3,6 @@ import groovy.transform.PackageScope; import whelk.JsonLd; import whelk.search2.querytree.*; -import whelk.util.Restrictions; import java.time.format.DateTimeParseException; import java.util.*; @@ -69,10 +68,10 @@ public Selector restrictByValue(Selector selector, String value) { return switch (selector) { case Property p -> restrictByValue(p, value); case Path path -> { - var narrowed = restrictByValue(path.last(), value); + var coerced = restrictByValue(path.last(), value); List newPath = new ArrayList<>(path.path()); newPath.removeLast(); - newPath.addAll(narrowed.path()); + newPath.addAll(coerced.path()); yield new Path(newPath, path.token()); } case Key k -> k; @@ -84,22 +83,19 @@ private boolean isRestrictedByValue(String propertyKey) { } private Property restrictByValue(Property property, String value) { - var narrowed = tryNarrow(property.name(), value); - if (narrowed != null) { - return new Property.NarrowedRestrictedProperty(property, narrowed, jsonLd); + var coercing = tryCoerce(property.name(), value); + if (coercing != null) { + return new Property.CoercingSubProperty(property, coercing, jsonLd); } return property; } - private String tryNarrow(String property, String value) { - var narrowedByValue = vocabMappings.propertiesRestrictedByValue() + private String tryCoerce(String property, String value) { + var coercingSubPropertyKey = vocabMappings.propertiesRestrictedByValue() .getOrDefault(property, Map.of()) .get(expandPrefixed(value)); - if (narrowedByValue != null) { - return narrowedByValue.getFirst(); - } else if (property.equals(Restrictions.CATEGORY)) { - // FIXME: Don't hardcode - return Restrictions.NONE_CATEGORY; + if (coercingSubPropertyKey != null) { + return coercingSubPropertyKey.getFirst(); } return null; } diff --git a/whelk-core/src/main/groovy/whelk/search2/Query.java b/whelk-core/src/main/groovy/whelk/search2/Query.java index 2b37300765..7d3dcb5da1 100644 --- a/whelk-core/src/main/groovy/whelk/search2/Query.java +++ b/whelk-core/src/main/groovy/whelk/search2/Query.java @@ -22,7 +22,6 @@ import whelk.search2.querytree.YearRange; import whelk.util.DocumentUtil; -import whelk.util.Restrictions; import java.util.ArrayList; import java.util.Collection; @@ -69,6 +68,10 @@ public class Query { private ReducedQueryTree fullQueryTree; + private static final String FIND_CATEGORY = "librissearch:findCategory"; + private static final String IDENTIFY_CATEGORY = "librissearch:identifyCategory"; + private static final String WORK_NONE_CATEGORY = "librissearch:workCategory"; + public enum SearchMode { SUGGEST, STANDARD_SEARCH, @@ -421,9 +424,9 @@ private static void addSliceToAggQuery(Map query, if (!slice.getShowIf().isEmpty()) { // Enable @none facet if find/identify/@none in query // TODO don't hardcode this if we decide it is what we want - if (ctx.selectedFacets().getSelected(Restrictions.FIND_CATEGORY).isEmpty() - && ctx.selectedFacets().getSelected(Restrictions.IDENTIFY_CATEGORY).isEmpty() - && ctx.selectedFacets().getSelected(Restrictions.NONE_CATEGORY).isEmpty()) { + if (ctx.selectedFacets().getSelected(FIND_CATEGORY).isEmpty() + && ctx.selectedFacets().getSelected(IDENTIFY_CATEGORY).isEmpty() + && ctx.selectedFacets().getSelected(WORK_NONE_CATEGORY).isEmpty()) { return; } } @@ -462,8 +465,8 @@ private static void addSliceToAggQuery(Map query, m.remove(slice.subSlice().propertyKey()); } // TODO don't hardcode this if we decide it is what we want - if (Restrictions.FIND_CATEGORY.equals(pKey) || Restrictions.IDENTIFY_CATEGORY.equals(pKey)) { - m.remove(Restrictions.NONE_CATEGORY); + if (FIND_CATEGORY.equals(pKey) || IDENTIFY_CATEGORY.equals(pKey)) { + m.remove(WORK_NONE_CATEGORY); } //if ("_categoryByCollection.@none".equals(pKey)) { // m.remove("_categoryByCollection.find"); @@ -477,7 +480,7 @@ private static void addSliceToAggQuery(Map query, query.put(field, filterWrap(aggs, property.name(), filter)); }); } - + private static Map buildCoreAqqQuery(String field, AppParams.Slice slice, AggContext ctx) { return buildCoreAqqQuery(field, slice, ctx, false); } @@ -709,9 +712,8 @@ public List> getObservations(AppParams.Slice slice, Value pa //List selected = selectedValue != null ? selectedValue : Collections.emptyList(); //addObservation.accept(qt.remove(selected).add(pv)); Predicate f = (Node n) -> n instanceof Condition c2 - && c2.selector().path().getLast() instanceof Property p - && "category".equals(p.queryKey()) - && p.isRestrictedSubProperty(); + && c2.selector() instanceof Property p + && (p.name().equals(WORK_NONE_CATEGORY) || (p instanceof Property.CoercingSubProperty coercing && coercing.getSuperProperty().name().equals(WORK_NONE_CATEGORY))); var qt2 = qt.removeAll(qt.findTopNodesByCondition(n -> f.test(n) || n instanceof Or or && or.children().stream().anyMatch(f))); if (selectedValue == null || !selectedValue.contains(c)) { @@ -787,14 +789,14 @@ public Map getSliceByDimension(List slices, Sel // Move @none to under selected find/identify // TODO don't hardcode this if we decide it is what we want - var none = s.remove(Restrictions.NONE_CATEGORY); + var none = s.remove(WORK_NONE_CATEGORY); if (none != null) { - var find = s.get(Restrictions.FIND_CATEGORY); + var find = s.get(FIND_CATEGORY); if (find != null) { DocumentUtil.traverse(find, (value, path) -> { - if (value instanceof Map m && m.containsKey("_selected") && m.get("_selected").equals(true) && !path.contains(Restrictions.NONE_CATEGORY)) { + if (value instanceof Map m && m.containsKey("_selected") && m.get("_selected").equals(true) && !path.contains(WORK_NONE_CATEGORY)) { var newV = new HashMap<>(m); - ((Map) newV.computeIfAbsent("sliceByDimension", k -> new HashMap<>())).put(Restrictions.NONE_CATEGORY, none); + ((Map) newV.computeIfAbsent("sliceByDimension", k -> new HashMap<>())).put(WORK_NONE_CATEGORY, none); return new DocumentUtil.Replace(newV); } return DocumentUtil.NOP; diff --git a/whelk-core/src/main/groovy/whelk/search2/SelectedFacets.java b/whelk-core/src/main/groovy/whelk/search2/SelectedFacets.java index 155d59d3ae..4f27c46b99 100644 --- a/whelk-core/src/main/groovy/whelk/search2/SelectedFacets.java +++ b/whelk-core/src/main/groovy/whelk/search2/SelectedFacets.java @@ -6,7 +6,6 @@ import whelk.search2.querytree.Or; import whelk.search2.querytree.QueryTree; import whelk.search2.querytree.YearRange; -import whelk.util.Restrictions; import java.util.ArrayList; import java.util.Collection; @@ -24,8 +23,8 @@ public class SelectedFacets { // TODO: don't hardcode private final Set radioProps = Set.of( - Restrictions.FIND_CATEGORY, - Restrictions.IDENTIFY_CATEGORY + "librissearch:findCategory", + "librissearch:identifyCategory" ); public SelectedFacets(QueryTree queryTree, List sliceList) { diff --git a/whelk-core/src/main/groovy/whelk/search2/SuggestQuery.java b/whelk-core/src/main/groovy/whelk/search2/SuggestQuery.java index be7daa56a9..be89c12cfc 100644 --- a/whelk-core/src/main/groovy/whelk/search2/SuggestQuery.java +++ b/whelk-core/src/main/groovy/whelk/search2/SuggestQuery.java @@ -29,8 +29,8 @@ public class SuggestQuery extends Query { put("Subject", List.of("subject")); put("Language", List.of("language", "originalLanguage")); put("BibliographicAgent", List.of("contributor", "subject")); - put("WorkCategory", List.of("category")); - put("InstanceCategory", List.of("hasInstanceCategory")); + put("WorkCategory", List.of("workCategory")); + put("InstanceCategory", List.of("instanceCategory")); }}; private record Edited(Node node, Token token) { diff --git a/whelk-core/src/main/groovy/whelk/search2/VocabMappings.java b/whelk-core/src/main/groovy/whelk/search2/VocabMappings.java index 581ea29c8a..0c43e8f027 100644 --- a/whelk-core/src/main/groovy/whelk/search2/VocabMappings.java +++ b/whelk-core/src/main/groovy/whelk/search2/VocabMappings.java @@ -8,7 +8,6 @@ import whelk.search2.querytree.Term; import whelk.search2.querytree.VocabTerm; import whelk.util.DocumentUtil; -import whelk.util.Restrictions; import java.util.ArrayList; import java.util.HashMap; @@ -63,7 +62,7 @@ public record VocabMappings( Map>> for example: [ - "category": [ + "workCategory": [ "https://id.kb.se/term/ktg/Literature": ["findCategory"], "https://id.kb.se/term/ktg/Software" : ["findCategory"], "https://id.kb.se/term/saogf/Poesi" : ["identifyCategory"] @@ -88,10 +87,15 @@ private static VocabMappings getMappings(Whelk whelk) { Map>> classes = new HashMap<>(); Map>> enums = new HashMap<>(); + List coercingProperties = new ArrayList<>(); + vocab.forEach((termKey, termDefinition) -> { String ns = getNs(termKey, systemVocabNs); if (isProperty(termDefinition)) { addAllMappings(termKey, ns, properties, whelk); + if (jsonLd.getCategoryMembers("librissearch:coercing").contains(termKey)) { + coercingProperties.add(termKey); + } } else if (isClass(termDefinition, jsonLd)) { addAllMappings(termKey, ns, classes, whelk); } else if (isEnum(termDefinition, jsonLd)) { @@ -101,7 +105,7 @@ private static VocabMappings getMappings(Whelk whelk) { addMapping(JsonLd.TYPE_KEY, RDF_TYPE, "rdf", properties); - return new VocabMappings(properties, classes, enums, getPropertiesRestrictedByValue(whelk)); + return new VocabMappings(properties, classes, enums, getPropertiesRestrictedByValue(whelk, coercingProperties)); } private static String getNs(String termKey, String systemVocabNs) { @@ -206,17 +210,21 @@ private static List getTypes(Map termDefinition) { return asList(termDefinition.get(JsonLd.TYPE_KEY)); } - private static Map>> getPropertiesRestrictedByValue(Whelk whelk) { + private static Map>> getPropertiesRestrictedByValue(Whelk whelk, List coercingProps) { JsonLd ld = whelk.getJsonld(); Map> groupedBySuperProp = new HashMap<>(); - Restrictions.NARROWS.forEach((narrowerProp, superProp) -> - groupedBySuperProp.computeIfAbsent(superProp, x -> new ArrayList<>()) - .add(narrowerProp)); + coercingProps.forEach(coercing -> { + String superProp = get(ld.vocabIndex, List.of(coercing, JsonLd.Rdfs.SUB_PROPERTY_OF, 0, ID_KEY), ""); + if (!superProp.isEmpty()) { + groupedBySuperProp.computeIfAbsent(ld.toTermKey(superProp), x -> new ArrayList<>()) + .add(coercing); + } + }); Map>> propertiesRestrictedByValue = new HashMap<>(); - groupedBySuperProp.forEach((superProp, narrowerProps) -> { + groupedBySuperProp.forEach((superProp, coercing) -> { var types = new HashSet(); ld.getRange(superProp).forEach(type -> { @@ -227,10 +235,10 @@ private static Map>> getPropertiesRestrictedByV for (String type : types) { for (var doc : whelk.getStorage().loadAllByType(type)) { var iri = doc.getThingIdentifiers().stream().findFirst().orElseThrow(); - for (var n : narrowerProps) { + for (var n : coercing) { var propDef = ld.vocabIndex.getOrDefault(n, Map.of()); Property.getObjectHasValueRestrictions(propDef, ld).forEach(hasValueRestriction -> { - var onProperty = hasValueRestriction.property(); + var onProperty = hasValueRestriction.onProperty(); var hasValue = hasValueRestriction.value(); var path = List.of(JsonLd.GRAPH_KEY, 1, onProperty.name()); Predicate hasMatchingValue = o -> switch (hasValue) { diff --git a/whelk-core/src/main/groovy/whelk/search2/querytree/Condition.java b/whelk-core/src/main/groovy/whelk/search2/querytree/Condition.java index de8c862599..410e0dc8de 100644 --- a/whelk-core/src/main/groovy/whelk/search2/querytree/Condition.java +++ b/whelk-core/src/main/groovy/whelk/search2/querytree/Condition.java @@ -171,12 +171,9 @@ private List getPrefilledFields(List path) { for (PathElement pe : path) { currentPath.add(pe); if (pe instanceof Property p && p.isRestrictedSubProperty() && !p.hasIndexKey()) { - for (Restrictions.OnProperty r : p.objectOnPropertyRestrictions()) { - // Support only HasValue restriction for now - if (r instanceof Restrictions.HasValue(Property property, Value v)) { - var restrictedPath = new Path(Stream.concat(currentPath.stream(), property.path().stream()).toList()); - prefilledFields.add(new Condition(restrictedPath, EQUALS, v)); - } + for (Restrictions.HasValue r : p.objectOnPropertyRestrictions()) { + var restrictedPath = new Path(Stream.concat(currentPath.stream(), r.onProperty().path().stream()).toList()); + prefilledFields.add(new Condition(restrictedPath, EQUALS, r.value())); } } } diff --git a/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java b/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java index 8a550c033d..e882fa822f 100644 --- a/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java +++ b/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java @@ -47,7 +47,7 @@ public non-sealed class Property extends PathElement { protected boolean isVocabTerm; protected Property superProperty; - protected List objectOnPropertyRestrictions; + protected List objectOnPropertyRestrictions; private static final String LIBRIS_SEARCH_NS = "librissearch:"; @@ -100,8 +100,8 @@ public static Property buildProperty(String propertyKey, JsonLd jsonLd, Key.Reco if (isShorthand(propDef)) { return new ShorthandProperty(propertyKey, jsonLd, queryKey); } - if (Restrictions.isNarrowingProperty(propertyKey)) { - return new NarrowedRestrictedProperty(propertyKey, jsonLd, queryKey); + if (isCoercing(propDef)) { + return new CoercingSubProperty(propertyKey, jsonLd, queryKey); } if (RDF_TYPE.equals(propertyKey)) { return new RdfType(jsonLd, queryKey); @@ -229,14 +229,12 @@ public boolean hasIndexKey() { return indexKey != null; } - public List objectOnPropertyRestrictions() { + public List objectOnPropertyRestrictions() { return objectOnPropertyRestrictions != null ? objectOnPropertyRestrictions : List.of(); } - protected List getObjectHasValueRestrictions(JsonLd jsonLd) { - return getObjectHasValueRestrictions(definition, jsonLd).stream() - .map(Restrictions.OnProperty.class::cast) - .toList(); + protected List getObjectHasValueRestrictions(JsonLd jsonLd) { + return getObjectHasValueRestrictions(definition, jsonLd); } public static List getObjectHasValueRestrictions(Map definition, JsonLd jsonLd) { @@ -372,6 +370,11 @@ private static boolean isComposite(Map definition) { return isCategory("https://id.kb.se/ns/librissearch/composite", definition); } + private static boolean isCoercing(Map definition) { + // FIXME: don't hardcode + return isCategory("https://id.kb.se/ns/librissearch/coercing", definition); + } + private static boolean isShorthand(Map definition) { // FIXME: don't hardcode return isCategory("https://id.kb.se/vocab/shorthand", definition); @@ -450,17 +453,21 @@ public Meta(JsonLd jsonLd, Key.RecognizedKey key) { } } - public static final class NarrowedRestrictedProperty extends Property { - public NarrowedRestrictedProperty(Property superProperty, String subPropertyKey, JsonLd jsonLd) { + public static final class CoercingSubProperty extends Property { + public CoercingSubProperty(Property superProperty, String subPropertyKey, JsonLd jsonLd) { super(subPropertyKey, jsonLd); this.superProperty = superProperty; } - public NarrowedRestrictedProperty(String subPropertyKey, JsonLd jsonLd, Key.RecognizedKey queryKey) { + public CoercingSubProperty(String subPropertyKey, JsonLd jsonLd, Key.RecognizedKey queryKey) { super(subPropertyKey, jsonLd, queryKey); this.superProperty = getSuperProperty(jsonLd); } + public Property getSuperProperty() { + return superProperty; + } + @Override public String queryKey() { return superProperty.queryKey(); @@ -471,6 +478,7 @@ public Map definition() { return superProperty.definition(); } + @Override public String esField() { if (hasIndexKey()) { return indexKey; @@ -482,11 +490,6 @@ public String esField() { public boolean isRestrictedSubProperty() { return true; } - - @Override - public boolean equals(Object obj) { - return super.equals(obj) || superProperty.equals(obj); - } } private static final class AnonymousProperty extends Property { @@ -517,7 +520,7 @@ public String esField() { } @Override - protected List getObjectHasValueRestrictions(JsonLd jsonLd) { + protected List getObjectHasValueRestrictions(JsonLd jsonLd) { var range = getUnambiguousRange(jsonLd); if (range != null) { /* diff --git a/whelk-core/src/main/groovy/whelk/util/Restrictions.java b/whelk-core/src/main/groovy/whelk/util/Restrictions.java index 8e44e611ee..2555b168cc 100644 --- a/whelk-core/src/main/groovy/whelk/util/Restrictions.java +++ b/whelk-core/src/main/groovy/whelk/util/Restrictions.java @@ -3,32 +3,10 @@ import whelk.search2.querytree.Property; import whelk.search2.querytree.Value; -import java.util.Collection; -import java.util.Map; - - public class Restrictions { - public static String CATEGORY = "category"; - public static String FIND_CATEGORY = "librissearch:findCategory"; - public static String IDENTIFY_CATEGORY = "librissearch:identifyCategory"; - public static String NONE_CATEGORY = "librissearch:noneCategory"; - - public sealed interface OnProperty permits HasNoneOfValues, HasValue { - } - - public record HasValue(Property property, Value value) implements OnProperty { - } - - public record HasNoneOfValues(Property property, Collection values) implements OnProperty { + public sealed interface OnProperty permits HasValue { } - public static boolean isNarrowingProperty(String propertyName) { - return NARROWS.containsKey(propertyName); + public record HasValue(Property onProperty, Value value) implements OnProperty { } - - public static final Map NARROWS = Map.of( - FIND_CATEGORY, CATEGORY, - IDENTIFY_CATEGORY, CATEGORY, - NONE_CATEGORY, CATEGORY - ); } diff --git a/whelk-core/src/test/groovy/whelk/search2/QuerySpec.groovy b/whelk-core/src/test/groovy/whelk/search2/QuerySpec.groovy index e926b93068..72b1f1ec1d 100644 --- a/whelk-core/src/test/groovy/whelk/search2/QuerySpec.groovy +++ b/whelk-core/src/test/groovy/whelk/search2/QuerySpec.groovy @@ -23,8 +23,8 @@ class QuerySpec extends Specification { "statistics": [ "sliceList": [ ["dimensionChain": ["findCategory"], "slice": ["dimensionChain": ["identifyCategory"]]], - ["dimensionChain": ["noneCategory"], "itemLimit": 100, "connective": "OR", "showIf": ["category"]], - ["dimensionChain": ["hasInstanceCategory"], "itemLimit": 100] + ["dimensionChain": ["workCategory"], "itemLimit": 100, "connective": "OR", "showIf": ["category"]], + ["dimensionChain": ["instanceCategory"], "itemLimit": 100] ] ] ] @@ -275,14 +275,14 @@ class QuerySpec extends Specification { "match_all": [:] ] ], - "@reverse.instanceOf.category.@id" : [ + "@reverse.instanceOf._categoryByCollection.@none.@id" : [ "aggs" : [ - "hasInstanceCategory" : [ + "librissearch:instanceCategory" : [ "terms" : [ "order" : [ "_count" : "desc" ], - "field" : "@reverse.instanceOf.category.@id", + "field" : "@reverse.instanceOf._categoryByCollection.@none.@id", "size" : 100 ] ] @@ -296,8 +296,8 @@ class QuerySpec extends Specification { def "build agg query for categories 2"() { given: - SelectedFacets selectedFacets = new SelectedFacets(new QueryTree('category:"https://id.kb.se/term/ktg/X"', disambiguate), appParams2.sliceList) - Map aggQuery = Query.buildAggQuery(appParams2.sliceList, jsonLd, [], esSettings, selectedFacets) + SelectedFacets selectedFacets = new SelectedFacets(new QueryTree('workCategory:"https://id.kb.se/term/ktg/X"', disambiguate), appParams2.sliceList) + Map aggQuery = Query.buildAggQuery(appParams2.sliceList, jsonLd, ['T2'], esSettings, selectedFacets) expect: aggQuery == [ @@ -337,7 +337,7 @@ class QuerySpec extends Specification { ], "_categoryByCollection.@none.@id" : [ "aggs" : [ - "librissearch:noneCategory" : [ + "librissearch:workCategory" : [ "terms" : [ "order" : [ "_count" : "desc" @@ -357,15 +357,15 @@ class QuerySpec extends Specification { ] ] ], - "@reverse.instanceOf.category.@id" : [ + "@reverse.instanceOf._categoryByCollection.@none.@id" : [ "aggs" : [ - "hasInstanceCategory" : [ + "librissearch:instanceCategory" : [ "terms" : [ "order" : [ "_count" : "desc" ], "size" : 100, - "field" : "@reverse.instanceOf.category.@id" + "field" : "@reverse.instanceOf._categoryByCollection.@none.@id" ] ] ], @@ -381,4 +381,92 @@ class QuerySpec extends Specification { ] ] } + + def "build agg query for categories 3"() { + given: + SelectedFacets selectedFacets = new SelectedFacets(new QueryTree('workCategory:"https://id.kb.se/term/ktg/Y"', disambiguate), appParams2.sliceList) + Map aggQuery = Query.buildAggQuery(appParams2.sliceList, jsonLd, ['T1'], esSettings, selectedFacets) + + expect: + aggQuery == [ + "instanceOf._categoryByCollection.find.@id" : [ + "aggs" : [ + "librissearch:findCategory" : [ + "terms" : [ + "size" : 10, + "field" : "instanceOf._categoryByCollection.find.@id", + "order" : [ + "_count" : "desc" + ] + ], + "aggs" : [ + "instanceOf._categoryByCollection.identify.@id" : [ + "aggs" : [ + "librissearch:identifyCategory" : [ + "terms" : [ + "size" : 10, + "field" : "instanceOf._categoryByCollection.identify.@id", + "order" : [ + "_count" : "desc" + ] + ] + ] + ], + "filter" : [ + "match_all" : [:] + ] + ] + ] + ] + ], + "filter" : [ + "match_all" : [:] + ] + ], + "instanceOf._categoryByCollection.@none.@id" : [ + "aggs" : [ + "librissearch:workCategory" : [ + "terms" : [ + "size" : 100, + "field" : "instanceOf._categoryByCollection.@none.@id", + "order" : [ + "_count" : "desc" + ] + ] + ] + ], + "filter" : [ + "bool" : [ + "filter" : [ + "term" : [ + "instanceOf._categoryByCollection.identify.@id" : "https://id.kb.se/term/ktg/Y" + ] + ] + ] + ] + ], + "_categoryByCollection.@none.@id" : [ + "aggs" : [ + "librissearch:instanceCategory" : [ + "terms" : [ + "size" : 100, + "field" : "_categoryByCollection.@none.@id", + "order" : [ + "_count" : "desc" + ] + ] + ] + ], + "filter" : [ + "bool" : [ + "filter" : [ + "term" : [ + "instanceOf._categoryByCollection.identify.@id" : "https://id.kb.se/term/ktg/Y" + ] + ] + ] + ] + ] + ] + } } diff --git a/whelk-core/src/test/groovy/whelk/search2/querytree/EsQueryTreeSpec.groovy b/whelk-core/src/test/groovy/whelk/search2/querytree/EsQueryTreeSpec.groovy index fd7f4a2b9f..19d81f5c1f 100644 --- a/whelk-core/src/test/groovy/whelk/search2/querytree/EsQueryTreeSpec.groovy +++ b/whelk-core/src/test/groovy/whelk/search2/querytree/EsQueryTreeSpec.groovy @@ -123,13 +123,13 @@ class EsQueryTreeSpec extends Specification { def "category ES query"() { given: - def q = 'type:T1x category:"https://id.kb.se/term/ktg/Y" category:("https://id.kb.se/term/ktg/A" OR "https://id.kb.se/term/ktg/B")' + def q = 'type:T2x workCategory:"https://id.kb.se/term/ktg/Y" workCategory:("https://id.kb.se/term/ktg/A" OR "https://id.kb.se/term/ktg/B")' QueryTree qt = new QueryTree(q, disambiguate) def appConfig = [ "statistics": [ "sliceList": [ ["dimensionChain": ["findCategory"], "slice": ["dimensionChain": ["identifyCategory"]]], - ["dimensionChain": ["noneCategory"], "itemLimit": 100, "connective": "OR", "showIf": ["category"]] + ["dimensionChain": ["workCategory"], "itemLimit": 100, "connective": "OR", "showIf": ["category"]] ] ] ] @@ -143,7 +143,7 @@ class EsQueryTreeSpec extends Specification { "bool": [ "filter": [ "term": [ - "@type": "T1x" + "@type": "T2x" ] ] ] @@ -151,6 +151,14 @@ class EsQueryTreeSpec extends Specification { esQueryTree.getPostFilter() == [ "bool": [ "must": [[ + "bool": [ + "filter": [ + "term": [ + "_categoryByCollection.identify.@id": "https://id.kb.se/term/ktg/Y" + ] + ] + ] + ], [ "bool": [ "should": [[ "bool": [ @@ -170,14 +178,6 @@ class EsQueryTreeSpec extends Specification { ] ]] ] - ], [ - "bool": [ - "filter": [ - "term": [ - "_categoryByCollection.identify.@id": "https://id.kb.se/term/ktg/Y" - ] - ] - ] ]] ] ] diff --git a/whelk-core/src/test/groovy/whelk/search2/querytree/QueryTreeSpec.groovy b/whelk-core/src/test/groovy/whelk/search2/querytree/QueryTreeSpec.groovy index 8f97574eda..4554301084 100644 --- a/whelk-core/src/test/groovy/whelk/search2/querytree/QueryTreeSpec.groovy +++ b/whelk-core/src/test/groovy/whelk/search2/querytree/QueryTreeSpec.groovy @@ -69,9 +69,16 @@ class QueryTreeSpec extends Specification { "(x OR y) p1:x" | "(x OR y) p1:x" "x OR y" | "x OR y" "(x OR y) z" | "(x OR y) z" - "findcategory:\"https://id.kb.se/term/ktg/X\"" | "category:\"https://id.kb.se/term/ktg/X\"" - "identifycategory:\"https://id.kb.se/term/ktg/Y\"" | "category:\"https://id.kb.se/term/ktg/Y\"" - "nonecategory:\"https://id.kb.se/term/ktg/Z\"" | "category:\"https://id.kb.se/term/ktg/Z\"" + "findcategory:\"https://id.kb.se/term/ktg/X\"" | "\"librissearch:workCategory\":\"https://id.kb.se/term/ktg/X\"" + "identifycategory:\"https://id.kb.se/term/ktg/Y\"" | "\"librissearch:workCategory\":\"https://id.kb.se/term/ktg/Y\"" + "workcategory:\"https://id.kb.se/term/ktg/X\"" | "workcategory:\"https://id.kb.se/term/ktg/X\"" + "workcategory:\"https://id.kb.se/term/ktg/Y\"" | "workcategory:\"https://id.kb.se/term/ktg/Y\"" + "workcategory:\"https://id.kb.se/term/ktg/Z\"" | "workcategory:\"https://id.kb.se/term/ktg/Z\"" + "workcategory:Y" | "workcategory:Y" + "workcategory:(X Y)" | "workcategory:(X Y)" + "instancecategory:\"https://id.kb.se/term/ktg/Z\"" | "instancecategory:\"https://id.kb.se/term/ktg/Z\"" + "instancecategory:X" | "instancecategory:X" + "instancecategory:(X Y)" | "instancecategory:(X Y)" "category:\"https://id.kb.se/term/ktg/X\"" | "category:\"https://id.kb.se/term/ktg/X\"" "category:\"https://id.kb.se/term/ktg/Y\"" | "category:\"https://id.kb.se/term/ktg/Y\"" "category:\"https://id.kb.se/term/ktg/Z\"" | "category:\"https://id.kb.se/term/ktg/Z\"" diff --git a/whelk-core/src/test/groovy/whelk/search2/querytree/TestData.groovy b/whelk-core/src/test/groovy/whelk/search2/querytree/TestData.groovy index 5b19a918c6..e231357abd 100644 --- a/whelk-core/src/test/groovy/whelk/search2/querytree/TestData.groovy +++ b/whelk-core/src/test/groovy/whelk/search2/querytree/TestData.groovy @@ -37,10 +37,10 @@ class TestData { 'p' : ['p', 'p1'] as Set, 'plabel' : ['p2', 'p3'] as Set, 'pp' : ['p3', 'p4'] as Set, - 'category' : ['category'] as Set, + 'workcategory' : ['librissearch:workCategory'] as Set, + 'instancecategory': ['librissearch:instanceCategory'] as Set, 'findcategory' : ['librissearch:findCategory'] as Set, 'identifycategory': ['librissearch:identifyCategory'] as Set, - 'nonecategory' : ['librissearch:noneCategory'] as Set, 'p3p1' : ['p3p1'] as Set, 'bibliography' : ['bibliography'] as Set, 'meta' : ['meta'] as Set @@ -64,7 +64,7 @@ class TestData { Stream.of(propertyMappings, classMappings, enumMappings).forEach(insertNamespace) def propertiesRestrictedByValue = [ - 'category': [ + 'librissearch:workCategory': [ 'https://id.kb.se/term/ktg/X': ['librissearch:findCategory'], 'https://id.kb.se/term/ktg/Y': ['librissearch:identifyCategory'] ] @@ -202,40 +202,31 @@ class TestData { 'range' : ['@id': 'T4'] ], [ - '@id' : 'category', - '@type': 'ObjectProperty' + '@id' : 'librissearch:workCategory', + '@type' : 'ObjectProperty', + 'domain' : ['@id': 'T2'], + 'ls:indexKey': '_categoryByCollection.@none' ], [ - '@id' : 'hasInstanceCategory', - 'category' : ['@id': "https://id.kb.se/vocab/shorthand"], - 'domain' : ['@id': 'T2'], - '@type' : 'ObjectProperty', - 'propertyChainAxiom': [['@list': [ - ['@id': 'hasInstance'], - ['@id': 'category'] - ]]] + '@id' : 'librissearch:instanceCategory', + '@type' : 'ObjectProperty', + 'domain' : ['@id': 'T1'], + 'ls:indexKey': '_categoryByCollection.@none' ], [ '@id' : 'librissearch:findCategory', '@type' : 'ObjectProperty', - 'subPropertyOf': [['@id': 'category']], - 'domain' : ['@id': 'T2'], + 'subPropertyOf': [['@id': 'librissearch:workCategory']], + 'category' : ['@id': "https://id.kb.se/ns/librissearch/coercing"], 'ls:indexKey' : '_categoryByCollection.find' ], [ '@id' : 'librissearch:identifyCategory', '@type' : 'ObjectProperty', - 'subPropertyOf': [['@id': 'category']], - 'domain' : ['@id': 'T2'], + 'subPropertyOf': [['@id': 'librissearch:workCategory']], + 'category' : ['@id': "https://id.kb.se/ns/librissearch/coercing"], 'ls:indexKey' : '_categoryByCollection.identify' ], - [ - '@id' : 'librissearch:noneCategory', - '@type' : 'ObjectProperty', - 'subPropertyOf': [['@id': 'category']], - 'domain' : ['@id': 'T2'], - 'ls:indexKey' : '_categoryByCollection.@none' - ], ['@id': 'textQuery', '@type': 'DatatypeProperty'], ['@id': 'rdf:type', '@type': 'ObjectProperty'], ['@id': 'meta', '@type': 'ObjectProperty'], @@ -272,15 +263,18 @@ class TestData { static def getEsMappings() { def mappings = [ 'properties': [ - 'p3' : ['type': 'nested'], - '@reverse.instanceOf.p3' : ['type': 'nested'], - '@type' : ['type': 'keyword'], - 'p2' : ['type': 'keyword'], - 'p3.p4.@id' : ['type': 'keyword'], - '_categoryByCollection.find.@id' : ['type': 'keyword'], - '_categoryByCollection.identify.@id': ['type': 'keyword'], - '_categoryByCollection.@none.@id' : ['type': 'keyword'], - '@reverse.instanceOf.category.@id' : ['type': 'keyword'] + 'p3' : ['type': 'nested'], + '@reverse.instanceOf.p3' : ['type': 'nested'], + '@type' : ['type': 'keyword'], + 'p2' : ['type': 'keyword'], + 'p3.p4.@id' : ['type': 'keyword'], + '_categoryByCollection.find.@id' : ['type': 'keyword'], + '_categoryByCollection.identify.@id' : ['type': 'keyword'], + '_categoryByCollection.@none.@id' : ['type': 'keyword'], + 'instanceOf._categoryByCollection.find.@id' : ['type': 'keyword'], + 'instanceOf._categoryByCollection.identify.@id' : ['type': 'keyword'], + 'instanceOf._categoryByCollection.@none.@id' : ['type': 'keyword'], + '@reverse.instanceOf._categoryByCollection.@none.@id': ['type': 'keyword'] ] ] return new EsMappings(mappings) From 789d7bb26ceccc9642aff457cda05846f8eb0a03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kalle=20W=C3=A5hlin?= Date: Tue, 24 Mar 2026 08:36:21 +0100 Subject: [PATCH 02/10] Exclude selectors incompatible with RDF subject types in agg query --- .../src/main/groovy/whelk/search2/Query.java | 2 +- .../whelk/search2/querytree/Condition.java | 2 +- .../groovy/whelk/search2/querytree/Key.java | 2 +- .../groovy/whelk/search2/querytree/Path.java | 22 ++++++++++------- .../whelk/search2/querytree/Property.java | 24 +++++++++++-------- .../whelk/search2/querytree/Selector.java | 2 +- 6 files changed, 32 insertions(+), 22 deletions(-) diff --git a/whelk-core/src/main/groovy/whelk/search2/Query.java b/whelk-core/src/main/groovy/whelk/search2/Query.java index 7d3dcb5da1..14d88707b6 100644 --- a/whelk-core/src/main/groovy/whelk/search2/Query.java +++ b/whelk-core/src/main/groovy/whelk/search2/Query.java @@ -436,7 +436,7 @@ private static void addSliceToAggQuery(Map query, throw new RuntimeException("Can't handle combined fields in aggs query"); } - property.getAltSelectors(ctx.jsonLd, ctx.rdfSubjectTypes).stream() + property.getAltSelectors(ctx.jsonLd, ctx.rdfSubjectTypes, false).stream() .map(s -> s.withPrependedMetaProperty(ctx.jsonLd)) .forEach(selector -> { String field = selector.esField(); diff --git a/whelk-core/src/main/groovy/whelk/search2/querytree/Condition.java b/whelk-core/src/main/groovy/whelk/search2/querytree/Condition.java index 410e0dc8de..06f06ea12f 100644 --- a/whelk-core/src/main/groovy/whelk/search2/querytree/Condition.java +++ b/whelk-core/src/main/groovy/whelk/search2/querytree/Condition.java @@ -146,7 +146,7 @@ public Type asTypeNode() { } private ExpandedNode expandWithAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes) { - List withAltSelectors = selector.getAltSelectors(jsonLd, rdfSubjectTypes).stream() + List withAltSelectors = selector.getAltSelectors(jsonLd, rdfSubjectTypes, true).stream() .map(s -> s.withPrependedMetaProperty(jsonLd)) .map(this::withSelector) .map(s -> s._expand(jsonLd)) diff --git a/whelk-core/src/main/groovy/whelk/search2/querytree/Key.java b/whelk-core/src/main/groovy/whelk/search2/querytree/Key.java index 5ae8a774d8..2ac04844af 100644 --- a/whelk-core/src/main/groovy/whelk/search2/querytree/Key.java +++ b/whelk-core/src/main/groovy/whelk/search2/querytree/Key.java @@ -31,7 +31,7 @@ public List path() { } @Override - public List getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes) { + public List getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes, boolean allowIncompatible) { return List.of(this); } diff --git a/whelk-core/src/main/groovy/whelk/search2/querytree/Path.java b/whelk-core/src/main/groovy/whelk/search2/querytree/Path.java index 2137cf5d0d..918a28251f 100644 --- a/whelk-core/src/main/groovy/whelk/search2/querytree/Path.java +++ b/whelk-core/src/main/groovy/whelk/search2/querytree/Path.java @@ -52,10 +52,16 @@ public List path() { } @Override - public List getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes) { - return getAltPaths(path(), jsonLd, rdfSubjectTypes).stream() - .map(l -> l.size() > 1 ? new Path(l) : l.getFirst()) - .toList(); + public List getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes, boolean allowIncompatible) { + List altSelectors = new ArrayList<>(); + getAltPaths(path(), jsonLd, rdfSubjectTypes, allowIncompatible).forEach(l -> { + if (l.size() == 1) { + altSelectors.add(l.getFirst()); + } else if (l.size() > 1) { + altSelectors.add(new Path(l)); + } + }); + return altSelectors; } @Override @@ -70,21 +76,21 @@ public Selector withPrependedMetaProperty(JsonLd jsonLd) { return new Path(newPath); } - private List> getAltPaths(List tail, JsonLd jsonLd, Collection rdfSubjectTypes) { + private List> getAltPaths(List tail, JsonLd jsonLd, Collection rdfSubjectTypes, boolean allowIncompatible) { if (tail.isEmpty()) { return List.of(List.of()); } var next = tail.getFirst(); var newTail = tail.subList(1, tail.size()); - var nextAltSelectors = next.getAltSelectors(jsonLd, rdfSubjectTypes); + var nextAltSelectors = next.getAltSelectors(jsonLd, rdfSubjectTypes, allowIncompatible); if (nextAltSelectors.isEmpty()) { // Indicates that an integral relation has been canceled out by its reverse // For example, when instanceOf is prepended to hasInstance.x // the integral property is dropped and only the tail (x) is kept - return getAltPaths(newTail, jsonLd, List.of()); + return getAltPaths(newTail, jsonLd, List.of(), allowIncompatible); } return nextAltSelectors.stream() - .flatMap(s -> getAltPaths(newTail, jsonLd, next.range()).stream() + .flatMap(s -> getAltPaths(newTail, jsonLd, next.range(), allowIncompatible).stream() .filter(altPath -> // Avoid creating alternative paths caused by inverse integral round-trips. // For example, if the original path is hasInstance.x, do not diff --git a/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java b/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java index e882fa822f..d023ce28f4 100644 --- a/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java +++ b/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java @@ -133,8 +133,8 @@ public List path() { } @Override - public List getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes) { - return _getAltSelectors(jsonLd, rdfSubjectTypes); + public List getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes, boolean allowIncompatible) { + return _getAltSelectors(jsonLd, rdfSubjectTypes, allowIncompatible); } @Override @@ -151,7 +151,7 @@ public boolean isValid() { @Override public boolean isType() { - return isRdfType(); + return isRdfType() || TYPE_KEY.equals(indexKey); } @Override @@ -296,8 +296,8 @@ public int hashCode() { return Objects.hash(toString()); } - private List _getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes) { - if (rdfSubjectTypes.isEmpty() || isPlatformTerm() || isRdfType()) { + private List _getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes, boolean allowIncompatible) { + if (isPlatformTerm() || isRdfType()) { return List.of(this); } @@ -324,7 +324,11 @@ private List _getAltSelectors(JsonLd jsonLd, Collection rdfSub }) .collect(Collectors.toList()); - if (altPaths.isEmpty() || isRecordProperty || rdfSubjectTypes.stream().anyMatch(t -> this.mayAppearOnType(t, jsonLd))) { + if (isRecordProperty || rdfSubjectTypes.stream().anyMatch(t -> this.mayAppearOnType(t, jsonLd))) { + altPaths.add(List.of(this)); + } + + if (altPaths.isEmpty() && allowIncompatible) { altPaths.add(List.of(this)); } @@ -566,9 +570,9 @@ public CompositeProperty(String name, JsonLd jsonLd, Key.RecognizedKey key) { } @Override - public List getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes) { + public List getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes, boolean allowIncompatible) { return getComponents(jsonLd).stream() - .flatMap(s -> s.getAltSelectors(jsonLd, rdfSubjectTypes).stream()) + .flatMap(s -> s.getAltSelectors(jsonLd, rdfSubjectTypes, allowIncompatible).stream()) .toList(); } @@ -618,8 +622,8 @@ public List path() { } @Override - public List getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes) { - return new Path(propertyChain).getAltSelectors(jsonLd, rdfSubjectTypes); + public List getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes, boolean allowIncompatible) { + return new Path(propertyChain).getAltSelectors(jsonLd, rdfSubjectTypes, allowIncompatible); } @Override diff --git a/whelk-core/src/main/groovy/whelk/search2/querytree/Selector.java b/whelk-core/src/main/groovy/whelk/search2/querytree/Selector.java index 827b2de9ce..d4aa1d3dd8 100644 --- a/whelk-core/src/main/groovy/whelk/search2/querytree/Selector.java +++ b/whelk-core/src/main/groovy/whelk/search2/querytree/Selector.java @@ -14,7 +14,7 @@ public sealed interface Selector permits Path, PathElement { List path(); - List getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes); + List getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes, boolean allowIncompatible); Selector withPrependedMetaProperty(JsonLd jsonLd); boolean isValid(); From f7aa49c8a00d465fbeb8b25a8e9e54e8e40ab937 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kalle=20W=C3=A5hlin?= Date: Tue, 24 Mar 2026 14:07:53 +0100 Subject: [PATCH 03/10] Remove awkward integral relation cancellation --- .../main/groovy/whelk/search2/querytree/Path.java | 13 ------------- .../groovy/whelk/search2/querytree/Property.java | 10 +--------- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/whelk-core/src/main/groovy/whelk/search2/querytree/Path.java b/whelk-core/src/main/groovy/whelk/search2/querytree/Path.java index 918a28251f..5e85a95e62 100644 --- a/whelk-core/src/main/groovy/whelk/search2/querytree/Path.java +++ b/whelk-core/src/main/groovy/whelk/search2/querytree/Path.java @@ -83,21 +83,8 @@ private List> getAltPaths(List tail, Js var next = tail.getFirst(); var newTail = tail.subList(1, tail.size()); var nextAltSelectors = next.getAltSelectors(jsonLd, rdfSubjectTypes, allowIncompatible); - if (nextAltSelectors.isEmpty()) { - // Indicates that an integral relation has been canceled out by its reverse - // For example, when instanceOf is prepended to hasInstance.x - // the integral property is dropped and only the tail (x) is kept - return getAltPaths(newTail, jsonLd, List.of(), allowIncompatible); - } return nextAltSelectors.stream() .flatMap(s -> getAltPaths(newTail, jsonLd, next.range(), allowIncompatible).stream() - .filter(altPath -> - // Avoid creating alternative paths caused by inverse integral round-trips. - // For example, if the original path is hasInstance.x, do not - // generate the alternative path x via instanceOf.hasInstance.x. - !(s instanceof Property p1 - && !altPath.isEmpty() && altPath.getFirst() instanceof Property p2 - && p1.isInverseOf(p2))) .map(altPath -> Stream.concat(s.path().stream(), altPath.stream()))) .map(Stream::toList) .toList(); diff --git a/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java b/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java index d023ce28f4..6e44b71909 100644 --- a/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java +++ b/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java @@ -313,15 +313,7 @@ private List _getAltSelectors(JsonLd jsonLd, Collection rdfSub List> altPaths = integralRelations.stream() .filter(followIntegralRelation) - .map(ir -> { - var altPath = new ArrayList<>(path()); - if (ir.isInverseOf(altPath.getFirst())) { - altPath.removeFirst(); - } else { - altPath.addFirst(ir); - } - return altPath; - }) + .map(ir -> Stream.concat(Stream.of(ir), path().stream()).toList()) .collect(Collectors.toList()); if (isRecordProperty || rdfSubjectTypes.stream().anyMatch(t -> this.mayAppearOnType(t, jsonLd))) { From fa98f46d9be18a41ff54f26a454e790a2968b534 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kalle=20W=C3=A5hlin?= Date: Tue, 24 Mar 2026 14:08:36 +0100 Subject: [PATCH 04/10] Adjust unit tests --- .../src/main/groovy/whelk/search2/querytree/Property.java | 2 +- .../test/groovy/whelk/search2/querytree/ConditionSpec.groovy | 2 -- .../test/groovy/whelk/search2/querytree/SelectorSpec.groovy | 4 +--- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java b/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java index 6e44b71909..4939d69a1e 100644 --- a/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java +++ b/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java @@ -297,7 +297,7 @@ public int hashCode() { } private List _getAltSelectors(JsonLd jsonLd, Collection rdfSubjectTypes, boolean allowIncompatible) { - if (isPlatformTerm() || isRdfType()) { + if (rdfSubjectTypes.isEmpty() || isPlatformTerm() || isRdfType()) { return List.of(this); } diff --git a/whelk-core/src/test/groovy/whelk/search2/querytree/ConditionSpec.groovy b/whelk-core/src/test/groovy/whelk/search2/querytree/ConditionSpec.groovy index fcfe3eafab..651632f351 100644 --- a/whelk-core/src/test/groovy/whelk/search2/querytree/ConditionSpec.groovy +++ b/whelk-core/src/test/groovy/whelk/search2/querytree/ConditionSpec.groovy @@ -100,10 +100,8 @@ class ConditionSpec extends Specification { "p1:v1" | ["T1"] | "instanceOf.p1:v1 OR p1:v1" "p1:v1" | ["T2"] | "hasInstance.p1:v1 OR p1:v1" "p1:v1" | ["T3"] | "p1:v1" - "hasInstance.p7:v7" | ["T1"] | "p7:v7" "hasInstance.p7:v7" | ["T2"] | "hasInstance.p7:v7" "instanceOf.p8:v8" | ["T1"] | "instanceOf.p8:v8" - "instanceOf.p8:v8" | ["T2"] | "p8:v8" "p5:x" | [] | "meta.p5:x" "meta.p5:x" | [] | "meta.p5:x" "bibliography:x" | ["T1"] | "instanceOf.meta.bibliography:x OR meta.bibliography:x" diff --git a/whelk-core/src/test/groovy/whelk/search2/querytree/SelectorSpec.groovy b/whelk-core/src/test/groovy/whelk/search2/querytree/SelectorSpec.groovy index 61c8cd3672..af9465a711 100644 --- a/whelk-core/src/test/groovy/whelk/search2/querytree/SelectorSpec.groovy +++ b/whelk-core/src/test/groovy/whelk/search2/querytree/SelectorSpec.groovy @@ -27,7 +27,7 @@ class SelectorSpec extends Specification { Selector p = ((Condition) QueryTreeBuilder.buildTree("$_p:v", disambiguate)).selector() expect: - p.getAltSelectors(jsonLd, types).collect { it.path().collect { it.toString() }.join(".") } == result + p.getAltSelectors(jsonLd, types, true).collect { it.path().collect { it.toString() }.join(".") } == result where: _p | types | result @@ -49,9 +49,7 @@ class SelectorSpec extends Specification { "p9" | ["T1", "T2"] | ["p9"] "p9" | ["T3"] | ["p9"] "hasInstance.p7" | ["T2"] | ["hasInstance.p7"] - "instanceOf.p8" | ["T2"] | ["p8"] "type" | ["T2"] | ["rdf:type"] - "hasInstance.p7" | ["T1"] | ["p7"] "instanceOf.p8" | ["T1"] | ["instanceOf.p8"] "type" | ["T1"] | ["rdf:type"] "p7.p14" | ["T2"] | ["hasInstance.p7.hasComponent.p14", "hasInstance.p7.p14"] From 86f46f576a6c20bc2747b7a8420ac0c2545d6143 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kalle=20W=C3=A5hlin?= Date: Fri, 27 Mar 2026 11:18:50 +0100 Subject: [PATCH 05/10] Omit librissearch: prefix from query keys --- .../src/main/groovy/whelk/search2/querytree/Property.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java b/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java index 4939d69a1e..cbafdbd43b 100644 --- a/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java +++ b/whelk-core/src/main/groovy/whelk/search2/querytree/Property.java @@ -117,7 +117,9 @@ public static Property buildProperty(String propertyKey, JsonLd jsonLd, Key.Reco @Override public String queryKey() { - return queryKey != null ? queryKey.queryKey() : name; + return queryKey != null + ? queryKey.queryKey() + : (name.startsWith(LIBRIS_SEARCH_NS) ? name.replace(LIBRIS_SEARCH_NS, "") : name); // FIXME } @Override From fa4ae4cc1435e42cbcc9d8889777fbe610c97cd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kalle=20W=C3=A5hlin?= Date: Tue, 31 Mar 2026 11:54:36 +0200 Subject: [PATCH 06/10] Fix unit tests --- .../test/groovy/whelk/search2/querytree/QueryTreeSpec.groovy | 4 ++-- .../src/test/groovy/whelk/search2/querytree/TestData.groovy | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/whelk-core/src/test/groovy/whelk/search2/querytree/QueryTreeSpec.groovy b/whelk-core/src/test/groovy/whelk/search2/querytree/QueryTreeSpec.groovy index 4554301084..885d503253 100644 --- a/whelk-core/src/test/groovy/whelk/search2/querytree/QueryTreeSpec.groovy +++ b/whelk-core/src/test/groovy/whelk/search2/querytree/QueryTreeSpec.groovy @@ -69,8 +69,8 @@ class QueryTreeSpec extends Specification { "(x OR y) p1:x" | "(x OR y) p1:x" "x OR y" | "x OR y" "(x OR y) z" | "(x OR y) z" - "findcategory:\"https://id.kb.se/term/ktg/X\"" | "\"librissearch:workCategory\":\"https://id.kb.se/term/ktg/X\"" - "identifycategory:\"https://id.kb.se/term/ktg/Y\"" | "\"librissearch:workCategory\":\"https://id.kb.se/term/ktg/Y\"" + "findcategory:\"https://id.kb.se/term/ktg/X\"" | "workCategory:\"https://id.kb.se/term/ktg/X\"" + "identifycategory:\"https://id.kb.se/term/ktg/Y\"" | "workCategory:\"https://id.kb.se/term/ktg/Y\"" "workcategory:\"https://id.kb.se/term/ktg/X\"" | "workcategory:\"https://id.kb.se/term/ktg/X\"" "workcategory:\"https://id.kb.se/term/ktg/Y\"" | "workcategory:\"https://id.kb.se/term/ktg/Y\"" "workcategory:\"https://id.kb.se/term/ktg/Z\"" | "workcategory:\"https://id.kb.se/term/ktg/Z\"" diff --git a/whelk-core/src/test/groovy/whelk/search2/querytree/TestData.groovy b/whelk-core/src/test/groovy/whelk/search2/querytree/TestData.groovy index 75bd5a08c5..5ff93d2cf6 100644 --- a/whelk-core/src/test/groovy/whelk/search2/querytree/TestData.groovy +++ b/whelk-core/src/test/groovy/whelk/search2/querytree/TestData.groovy @@ -271,7 +271,7 @@ class TestData { 'properties': [ 'p3' : ['type': 'nested'], '@reverse.instanceOf.p3' : ['type': 'nested'], - 'p15' : ['type': 'nested', "include_in_parent": true] + 'p15' : ['type': 'nested', "include_in_parent": true], '@type' : ['type': 'keyword'], 'p2' : ['type': 'keyword'], 'p3.p4.@id' : ['type': 'keyword'], From b303d2ed7f035824a87947ec9eb4151ece9ec314 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kalle=20W=C3=A5hlin?= Date: Thu, 16 Apr 2026 17:56:21 +0200 Subject: [PATCH 07/10] Reintroduce noneCategory and refactor SelectedFacets --- .../groovy/whelk/search2/Disambiguate.java | 3 + .../src/main/groovy/whelk/search2/Query.java | 43 ++--- .../groovy/whelk/search2/SelectedFacets.java | 166 ++++++++++-------- .../groovy/whelk/search2/QuerySpec.groovy | 42 ++--- .../search2/querytree/EsQueryTreeSpec.groovy | 18 +- .../whelk/search2/querytree/TestData.groovy | 12 +- 6 files changed, 158 insertions(+), 126 deletions(-) diff --git a/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java b/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java index 1f64c23bf2..0a40989e61 100644 --- a/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java +++ b/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java @@ -86,6 +86,9 @@ private Property restrictByValue(Property property, String value) { var coercing = tryCoerce(property.name(), value); if (coercing != null) { return new Property.CoercingSubProperty(property, coercing, jsonLd); + } else if (property.name().equals("librissearch:workCategory")) { + // FIXME: Don't hardcode + return new Property.CoercingSubProperty(property, "librissearch:noneCategory", jsonLd); } return property; } diff --git a/whelk-core/src/main/groovy/whelk/search2/Query.java b/whelk-core/src/main/groovy/whelk/search2/Query.java index ef93a53db6..a6d8be8298 100644 --- a/whelk-core/src/main/groovy/whelk/search2/Query.java +++ b/whelk-core/src/main/groovy/whelk/search2/Query.java @@ -19,7 +19,6 @@ import whelk.search2.querytree.QueryTree; import whelk.search2.querytree.ReducedQueryTree; import whelk.search2.querytree.Resource; -import whelk.search2.querytree.Type; import whelk.search2.querytree.Value; import whelk.search2.querytree.YearRange; @@ -71,9 +70,11 @@ public class Query { private ReducedQueryTree fullQueryTree; + private static final String WORK_CATEGORY = "librissearch:workCategory"; + private static final String FIND_CATEGORY = "librissearch:findCategory"; private static final String IDENTIFY_CATEGORY = "librissearch:identifyCategory"; - private static final String WORK_NONE_CATEGORY = "librissearch:workCategory"; + private static final String NONE_CATEGORY = "librissearch:noneCategory"; public enum SearchMode { SUGGEST, @@ -456,14 +457,13 @@ private static void addSliceToAggQuery(Map query, Property property = slice.getProperty(); - if (!slice.getShowIf().isEmpty()) { + if (!slice.getShowIf().isEmpty() + && ctx.selectedFacets.isInactive(FIND_CATEGORY) + && ctx.selectedFacets.isInactive(IDENTIFY_CATEGORY) + && ctx.selectedFacets.isInactive(NONE_CATEGORY)) { // Enable @none facet if find/identify/@none in query // TODO don't hardcode this if we decide it is what we want - if (ctx.selectedFacets().getSelected(FIND_CATEGORY).isEmpty() - && ctx.selectedFacets().getSelected(IDENTIFY_CATEGORY).isEmpty() - && ctx.selectedFacets().getSelected(WORK_NONE_CATEGORY).isEmpty()) { - return; - } + return; } if (property.isRestrictedSubProperty() && !property.hasIndexKey()) { @@ -501,7 +501,7 @@ private static void addSliceToAggQuery(Map query, } // TODO don't hardcode this if we decide it is what we want if (FIND_CATEGORY.equals(pKey) || IDENTIFY_CATEGORY.equals(pKey)) { - m.remove(WORK_NONE_CATEGORY); + m.remove(NONE_CATEGORY); } //if ("_categoryByCollection.@none".equals(pKey)) { // m.remove("_categoryByCollection.find"); @@ -685,14 +685,16 @@ public List> getObservations(AppParams.Slice slice, Value pa var property = slice.getProperty(); String propertyKey = slice.propertyKey(); - List> observations = new ArrayList<>(); - Connective connective = selectedFacets.getConnective(propertyKey); - - QueryTree qt = selectedFacets.isRangeFilter(propertyKey) + QueryTree qt = slice.isRange() ? qTree.removeAll(selectedFacets.getRangeSelected(propertyKey)) : qTree; + List selected = selectedFacets.getSelected(propertyKey); + Connective connective = selectedFacets.inferConnective(propertyKey).orElse(slice.defaultConnective()); + + List> observations = new ArrayList<>(); + this.buckets.entrySet() .stream() // TODO only do this for nested aggs of the same property etc etc @@ -715,7 +717,7 @@ public List> getObservations(AppParams.Slice slice, Value pa // TODO boolean isSelected = selectedValue != null && !selectedValue.isEmpty() ? selectedValue.stream().anyMatch(n -> n instanceof Condition c2 && c2.value() instanceof Link l && v instanceof Link l2 && l.iri().equals(l2.iri())) - : selectedFacets.isSelected(c, propertyKey); + : selected.contains(c); Consumer addObservation = alteredTree -> { Map observation = new LinkedHashMap<>(); @@ -750,7 +752,7 @@ public List> getObservations(AppParams.Slice slice, Value pa //addObservation.accept(qt.remove(selected).add(pv)); Predicate f = (Node n) -> n instanceof Condition c2 && c2.selector() instanceof Property p - && (p.name().equals(WORK_NONE_CATEGORY) || (p instanceof Property.CoercingSubProperty coercing && coercing.getSuperProperty().name().equals(WORK_NONE_CATEGORY))); + && p instanceof Property.CoercingSubProperty coercing && coercing.getSuperProperty().name().equals(WORK_CATEGORY); var qt2 = qt.removeAll(qt.findTopNodesByCondition(n -> f.test(n) || n instanceof Or or && or.children().stream().anyMatch(f))); if (selectedValue == null || !selectedValue.contains(c)) { @@ -761,7 +763,6 @@ public List> getObservations(AppParams.Slice slice, Value pa return; } - var selected = selectedFacets.getSelected(propertyKey); if (isSelected) { selected.stream() .filter(c::equals) @@ -826,14 +827,14 @@ public Map getSliceByDimension(List slices, Sel // Move @none to under selected find/identify // TODO don't hardcode this if we decide it is what we want - var none = s.remove(WORK_NONE_CATEGORY); + var none = s.remove(NONE_CATEGORY); if (none != null) { var find = s.get(FIND_CATEGORY); if (find != null) { DocumentUtil.traverse(find, (value, path) -> { - if (value instanceof Map m && m.containsKey("_selected") && m.get("_selected").equals(true) && !path.contains(WORK_NONE_CATEGORY)) { + if (value instanceof Map m && m.containsKey("_selected") && m.get("_selected").equals(true) && !path.contains(NONE_CATEGORY)) { var newV = new HashMap<>(m); - ((Map) newV.computeIfAbsent("sliceByDimension", k -> new HashMap<>())).put(WORK_NONE_CATEGORY, none); + ((Map) newV.computeIfAbsent("sliceByDimension", k -> new HashMap<>())).put(NONE_CATEGORY, none); return new DocumentUtil.Replace(newV); } return DocumentUtil.NOP; @@ -881,13 +882,13 @@ private Map getSliceByDimension(List slices, Se var sliceNode = new LinkedHashMap<>(); var observations = sliceResult.getObservations(slice, parentValue, mySelectedValue, selectedFacets); if (!observations.isEmpty() || parentValue != null) { - if (selectedFacets.isRangeFilter(propertyKey)) { + if (slice.isRange()) { sliceNode.put("search", getRangeTemplate(property)); } sliceNode.put("dimension", property.name()); sliceNode.put("observation", observations); sliceNode.put("maxItems", slice.size()); - sliceNode.put("_connective", selectedFacets.getConnective(propertyKey).name()); + sliceNode.put("_connective", selectedFacets.inferConnective(propertyKey).orElse(slice.defaultConnective())); result.put(property.name(), sliceNode); } }); diff --git a/whelk-core/src/main/groovy/whelk/search2/SelectedFacets.java b/whelk-core/src/main/groovy/whelk/search2/SelectedFacets.java index 4f27c46b99..61be704e94 100644 --- a/whelk-core/src/main/groovy/whelk/search2/SelectedFacets.java +++ b/whelk-core/src/main/groovy/whelk/search2/SelectedFacets.java @@ -7,19 +7,21 @@ import whelk.search2.querytree.QueryTree; import whelk.search2.querytree.YearRange; -import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.function.Predicate; public class SelectedFacets { private final Map> selectedByPropertyKey = new HashMap<>(); - private final Map propertyKeyToConnective = new HashMap<>(); - private final Set rangeProps = new HashSet<>(); + private final Map> multiSelectedByPropertyKey = new HashMap<>(); + private final Map> radioSelectedByPropertyKey = new HashMap<>(); + + private final Set selectable = new HashSet<>(); // TODO: don't hardcode private final Set radioProps = Set.of( @@ -28,60 +30,59 @@ public class SelectedFacets { ); public SelectedFacets(QueryTree queryTree, List sliceList) { - //FIXME - for (String radioProp : radioProps) { - selectedByPropertyKey.put(radioProp, new ArrayList<>()); - } - init(queryTree, sliceList); } public boolean isSelectable(String propertyKey) { - return selectedByPropertyKey.containsKey(propertyKey); + return selectable.contains(propertyKey); } - public boolean isMultiOrRadio(String propertyKey) { - return isMultiSelectable(propertyKey) || isRadioButton(propertyKey); + public boolean isInactive(String propertyKey) { + return getSelected(propertyKey).isEmpty(); } - public boolean isRadioButton(String propertyKey) { - return isSelectable(propertyKey) && radioProps.contains(propertyKey); + public boolean isMultiOrRadio(String propertyKey) { + return isOrSelected(propertyKey) || anyRadioSelected(); } - private boolean isMultiSelectable(String propertyKey) { - return isSelectable(propertyKey) && Query.Connective.OR.equals(propertyKeyToConnective.get(propertyKey)); + public boolean isRadioButton(String propertyKey) { + return radioProps.contains(propertyKey); } public List getSelected(String propertyKey) { - return isSelectable(propertyKey) ? selectedByPropertyKey.get(propertyKey) : List.of(); - } - - public boolean isSelected(Condition condition, String propertyKey) { - return isSelectable(propertyKey) && selectedByPropertyKey.get(propertyKey).contains(condition); + if (isAndSelected(propertyKey)) { + return getAndSelected(propertyKey); + } + if (isOrSelected(propertyKey)) { + return getOrSelected(propertyKey); + } + if (isRadioSelected(propertyKey)) { + return getRadioSelected(propertyKey); + } + return List.of(); } - public Map> getAllMultiOrRadioSelected() { - Map> result = new HashMap<>(); - selectedByPropertyKey.forEach((pKey, selected) -> { - if (isMultiOrRadio(pKey) && !getSelected(pKey).isEmpty()) { - result.put(pKey, getSelected(pKey)); - } - }); - return result; + public Optional inferConnective(String propertyKey) { + if (isAndSelected(propertyKey)) { + return Optional.of(Query.Connective.AND); + } + if (isOrSelected(propertyKey)) { + return Optional.of(Query.Connective.OR); + } + return Optional.empty(); } - public List getRangeSelected(String propertyKey) { + List getRangeSelected(String propertyKey) { return getSelected(propertyKey).stream() .filter(c -> c.operator().isRange() || c.value() instanceof YearRange) .toList(); } - public Query.Connective getConnective(String propertyKey) { - return propertyKeyToConnective.getOrDefault(propertyKey, Query.Connective.AND); - } - - public boolean isRangeFilter(String propertyKey) { - return rangeProps.contains(propertyKey); + public Map> getAllMultiOrRadioSelected() { + Map> result = new HashMap<>(); + result.putAll(multiSelectedByPropertyKey); + result.putAll(radioSelectedByPropertyKey); + return result; } public static QueryTree buildMultiSelectedTree(Collection> multiSelected) { @@ -97,54 +98,73 @@ public static QueryTree buildMultiSelectedTree(Collection isProperty = n -> n instanceof Condition c && c.selector().equals(property); - Predicate hasEqualsOp = n -> ((Condition) n).operator().equals(Operator.EQUALS); - Predicate isPropertyEquals = n -> isProperty.test(n) && hasEqualsOp.test(n); + private boolean anyRadioSelected() { + return !radioSelectedByPropertyKey.isEmpty(); + } - List allNodesWithProperty = queryTree.allDescendants().filter(isProperty).map(Condition.class::cast).toList(); + private List getAndSelected(String propertyKey) { + return selectedByPropertyKey.get(propertyKey); + } - if (slice.subSlice() != null) { - addSlice(slice.subSlice(), queryTree); - } + private List getOrSelected(String propertyKey) { + return multiSelectedByPropertyKey.get(propertyKey); + } - if (allNodesWithProperty.isEmpty()) { - selectedByPropertyKey.put(pKey, List.of()); - propertyKeyToConnective.put(pKey, slice.defaultConnective()); - return; - } + private List getRadioSelected(String propertyKey) { + return radioSelectedByPropertyKey.get(propertyKey); + } + + private void addSlice(AppParams.Slice slice, QueryTree queryTree) { + if (slice.subSlice() != null) { + addSlice(slice.subSlice(), queryTree); + } + + Predicate isProperty = n -> n instanceof Condition c && slice.getProperty().equals(c.selector()); - List selected = queryTree.getTopNodes().stream() - .filter(slice.isRange() ? isProperty : isPropertyEquals) - .map(Condition.class::cast) - .toList(); - - List> multiSelected = queryTree.getTopNodesOfType(Or.class).stream() - .map(Or::children) - .filter(orChildren -> orChildren.stream().allMatch(isPropertyEquals)) - .map(selectedConditions -> selectedConditions.stream().map(Condition.class::cast).toList()) - .toList(); - - if (selected.isEmpty() && multiSelected.size() == 1) { - var selectedConditions = multiSelected.getFirst(); - if (selectedConditions.equals(allNodesWithProperty)) { - selectedByPropertyKey.put(pKey, selectedConditions); - propertyKeyToConnective.put(pKey, Query.Connective.OR); + List selected = queryTree.getTopNodes().stream() + .filter(isProperty) + .map(Condition.class::cast) + .toList(); + + List> multiSelected = queryTree.getTopNodesOfType(Or.class).stream() + .map(Or::children) + .filter(orChildren -> orChildren.stream().allMatch(isProperty)) + .map(selectedConditions -> selectedConditions.stream().map(Condition.class::cast).toList()) + .toList(); + + String pKey = slice.getProperty().name(); + + if (selected.isEmpty() && multiSelected.size() == 1 && !isRadioButton(pKey)) { + multiSelectedByPropertyKey.put(pKey, multiSelected.getFirst()); + } else if (selected.size() == 1 && multiSelected.isEmpty()) { + if (isRadioButton(pKey)) { + radioSelectedByPropertyKey.put(pKey, selected); + } else { + switch (slice.defaultConnective()) { + case AND -> selectedByPropertyKey.put(pKey, selected); + case OR -> multiSelectedByPropertyKey.put(pKey, selected); } - } else if (multiSelected.isEmpty() && selected.equals(allNodesWithProperty)) { - selectedByPropertyKey.put(pKey, selected); - propertyKeyToConnective.put(pKey, selected.size() == 1 ? slice.defaultConnective() : Query.Connective.AND); } + } else if (selected.size() > 1 && multiSelected.isEmpty()) { + selectedByPropertyKey.put(pKey, selected); + } else if (!selected.isEmpty() || !multiSelected.isEmpty()) { + // Can't be mirrored in facets + return; } + + selectable.add(pKey); } private void init(QueryTree queryTree, List sliceList) { diff --git a/whelk-core/src/test/groovy/whelk/search2/QuerySpec.groovy b/whelk-core/src/test/groovy/whelk/search2/QuerySpec.groovy index 72b1f1ec1d..b67b0a6b54 100644 --- a/whelk-core/src/test/groovy/whelk/search2/QuerySpec.groovy +++ b/whelk-core/src/test/groovy/whelk/search2/QuerySpec.groovy @@ -23,7 +23,7 @@ class QuerySpec extends Specification { "statistics": [ "sliceList": [ ["dimensionChain": ["findCategory"], "slice": ["dimensionChain": ["identifyCategory"]]], - ["dimensionChain": ["workCategory"], "itemLimit": 100, "connective": "OR", "showIf": ["category"]], + ["dimensionChain": ["noneCategory"], "itemLimit": 100, "connective": "OR", "showIf": ["category"]], ["dimensionChain": ["instanceCategory"], "itemLimit": 100] ] ] @@ -242,53 +242,53 @@ class QuerySpec extends Specification { expect: aggQuery == [ "_categoryByCollection.find.@id" : [ + "filter" : [ + "match_all" : [:] + ], "aggs" : [ "librissearch:findCategory" : [ "terms" : [ + "size" : 10, + "field" : "_categoryByCollection.find.@id", "order" : [ "_count" : "desc" - ], - "field" : "_categoryByCollection.find.@id", - "size" : 10 + ] ], "aggs" : [ "_categoryByCollection.identify.@id" : [ + "filter" : [ + "match_all" : [:] + ], "aggs" : [ "librissearch:identifyCategory" : [ "terms" : [ + "size" : 10, + "field" : "_categoryByCollection.identify.@id", "order" : [ "_count" : "desc" - ], - "field" : "_categoryByCollection.identify.@id", - "size" : 10 + ] ] ] - ], - "filter" : [ - "match_all": [:] ] ] ] ] - ], - "filter" : [ - "match_all": [:] ] ], "@reverse.instanceOf._categoryByCollection.@none.@id" : [ + "filter" : [ + "match_all" : [:] + ], "aggs" : [ "librissearch:instanceCategory" : [ "terms" : [ + "size" : 100, + "field" : "@reverse.instanceOf._categoryByCollection.@none.@id", "order" : [ "_count" : "desc" - ], - "field" : "@reverse.instanceOf._categoryByCollection.@none.@id", - "size" : 100 + ] ] ] - ], - "filter" : [ - "match_all": [:] ] ] ] @@ -337,7 +337,7 @@ class QuerySpec extends Specification { ], "_categoryByCollection.@none.@id" : [ "aggs" : [ - "librissearch:workCategory" : [ + "librissearch:noneCategory" : [ "terms" : [ "order" : [ "_count" : "desc" @@ -425,7 +425,7 @@ class QuerySpec extends Specification { ], "instanceOf._categoryByCollection.@none.@id" : [ "aggs" : [ - "librissearch:workCategory" : [ + "librissearch:noneCategory" : [ "terms" : [ "size" : 100, "field" : "instanceOf._categoryByCollection.@none.@id", diff --git a/whelk-core/src/test/groovy/whelk/search2/querytree/EsQueryTreeSpec.groovy b/whelk-core/src/test/groovy/whelk/search2/querytree/EsQueryTreeSpec.groovy index 1d39883620..5aea057a09 100644 --- a/whelk-core/src/test/groovy/whelk/search2/querytree/EsQueryTreeSpec.groovy +++ b/whelk-core/src/test/groovy/whelk/search2/querytree/EsQueryTreeSpec.groovy @@ -129,7 +129,7 @@ class EsQueryTreeSpec extends Specification { "statistics": [ "sliceList": [ ["dimensionChain": ["findCategory"], "slice": ["dimensionChain": ["identifyCategory"]]], - ["dimensionChain": ["workCategory"], "itemLimit": 100, "connective": "OR", "showIf": ["category"]] + ["dimensionChain": ["noneCategory"], "itemLimit": 100, "connective": "OR", "showIf": ["category"]] ] ] ] @@ -151,14 +151,6 @@ class EsQueryTreeSpec extends Specification { esQueryTree.getPostFilter() == [ "bool": [ "must": [[ - "bool": [ - "filter": [ - "term": [ - "_categoryByCollection.identify.@id": "https://id.kb.se/term/ktg/Y" - ] - ] - ] - ], [ "bool": [ "should": [[ "bool": [ @@ -177,6 +169,14 @@ class EsQueryTreeSpec extends Specification { ] ] ]] + ]], + [ + "bool": [ + "filter": [ + "term": [ + "_categoryByCollection.identify.@id": "https://id.kb.se/term/ktg/Y" + ] + ] ] ]] ] diff --git a/whelk-core/src/test/groovy/whelk/search2/querytree/TestData.groovy b/whelk-core/src/test/groovy/whelk/search2/querytree/TestData.groovy index 5ff93d2cf6..007116c172 100644 --- a/whelk-core/src/test/groovy/whelk/search2/querytree/TestData.groovy +++ b/whelk-core/src/test/groovy/whelk/search2/querytree/TestData.groovy @@ -42,6 +42,7 @@ class TestData { 'instancecategory': ['librissearch:instanceCategory'] as Set, 'findcategory' : ['librissearch:findCategory'] as Set, 'identifycategory': ['librissearch:identifyCategory'] as Set, + 'nonecategory' : ['librissearch:noneCategory'] as Set, 'p3p1' : ['p3p1'] as Set, 'bibliography' : ['bibliography'] as Set, 'meta' : ['meta'] as Set @@ -208,9 +209,9 @@ class TestData { ], [ '@id' : 'librissearch:workCategory', + 'category' : ['@id': "https://id.kb.se/ns/librissearch/composite"], '@type' : 'ObjectProperty', - 'domain' : ['@id': 'T2'], - 'ls:indexKey': '_categoryByCollection.@none' + 'domain' : ['@id': 'T2'] ], [ '@id' : 'librissearch:instanceCategory', @@ -232,6 +233,13 @@ class TestData { 'category' : ['@id': "https://id.kb.se/ns/librissearch/coercing"], 'ls:indexKey' : '_categoryByCollection.identify' ], + [ + '@id' : 'librissearch:noneCategory', + '@type' : 'ObjectProperty', + 'subPropertyOf': [['@id': 'librissearch:workCategory']], + 'category' : ['@id': "https://id.kb.se/ns/librissearch/coercing"], + 'ls:indexKey' : '_categoryByCollection.@none' + ], ['@id': 'textQuery', '@type': 'DatatypeProperty'], ['@id': 'rdf:type', '@type': 'ObjectProperty'], ['@id': 'meta', '@type': 'ObjectProperty'], From 75f9d5785134a7b29ed3230869621c9ce2a41c7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kalle=20W=C3=A5hlin?= Date: Thu, 16 Apr 2026 18:30:55 +0200 Subject: [PATCH 08/10] Restrict by value only when the value is a Resource --- whelk-core/src/main/groovy/whelk/search2/Disambiguate.java | 2 +- .../groovy/whelk/search2/querytree/QueryTreeBuilder.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java b/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java index 0a40989e61..aac426fa99 100644 --- a/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java +++ b/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java @@ -96,7 +96,7 @@ private Property restrictByValue(Property property, String value) { private String tryCoerce(String property, String value) { var coercingSubPropertyKey = vocabMappings.propertiesRestrictedByValue() .getOrDefault(property, Map.of()) - .get(expandPrefixed(value)); + .get(value); if (coercingSubPropertyKey != null) { return coercingSubPropertyKey.getFirst(); } diff --git a/whelk-core/src/main/groovy/whelk/search2/querytree/QueryTreeBuilder.java b/whelk-core/src/main/groovy/whelk/search2/querytree/QueryTreeBuilder.java index e884cc1962..5ef4817000 100644 --- a/whelk-core/src/main/groovy/whelk/search2/querytree/QueryTreeBuilder.java +++ b/whelk-core/src/main/groovy/whelk/search2/querytree/QueryTreeBuilder.java @@ -135,10 +135,10 @@ private static Node buildFromCode(Ast.Code c, Disambiguate disambiguate, Selecto private static Condition buildCondition(Selector selector, Operator operator, Ast.Leaf leaf, Disambiguate disambiguate) { Token token = getToken(leaf.value()); - if (disambiguate.isRestrictedByValue(selector)) { - selector = disambiguate.restrictByValue(selector, token.value()); - } Value value = disambiguate.mapValueForSelector(selector, token).orElse(new FreeText(token)); + if (value instanceof Resource r && disambiguate.isRestrictedByValue(selector)) { + selector = disambiguate.restrictByValue(selector, r.jsonForm()); + } Condition condition = new Condition(selector, operator, value); return condition.isTypeNode() ? condition.asTypeNode() : condition; } From 1c414914d240ec490b78afdfbff00be7553ebc21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kalle=20W=C3=A5hlin?= Date: Fri, 17 Apr 2026 14:38:28 +0200 Subject: [PATCH 09/10] Fix immutable Map --- whelk-core/src/main/groovy/whelk/search2/ObjectQuery.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/whelk-core/src/main/groovy/whelk/search2/ObjectQuery.java b/whelk-core/src/main/groovy/whelk/search2/ObjectQuery.java index acabee68ce..6cede631e7 100644 --- a/whelk-core/src/main/groovy/whelk/search2/ObjectQuery.java +++ b/whelk-core/src/main/groovy/whelk/search2/ObjectQuery.java @@ -91,7 +91,7 @@ protected EsQuery doGetEsQuery() { } List subjectTypes = Stream.concat(givenSubjectTypes.stream(), inferredSubjectTypes.stream()).toList(); - var aggQuery = getEsAggQuery(subjectTypes); + var aggQuery = new LinkedHashMap<>(getEsAggQuery(subjectTypes)); aggQuery.putAll(getPAggQuery(predicateToSubjectTypes)); esQueryDsl.put("aggs", aggQuery); From bd89a4f489b4ddb94abad4cca9c788aefc6f986b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olov=20Ylinenp=C3=A4=C3=A4?= Date: Fri, 17 Apr 2026 16:00:00 +0200 Subject: [PATCH 10/10] Use constant --- whelk-core/src/main/groovy/whelk/search2/Disambiguate.java | 6 ++++-- whelk-core/src/main/groovy/whelk/search2/Query.java | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java b/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java index aac426fa99..3d0afda0f2 100644 --- a/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java +++ b/whelk-core/src/main/groovy/whelk/search2/Disambiguate.java @@ -13,6 +13,8 @@ import static whelk.JsonLd.LD_KEYS; import static whelk.JsonLd.VOCAB_KEY; import static whelk.JsonLd.looksLikeIri; +import static whelk.search2.Query.NONE_CATEGORY; +import static whelk.search2.Query.WORK_CATEGORY; import static whelk.search2.QueryUtil.encodeUri; import static whelk.search2.VocabMappings.expandPrefixed; @@ -86,9 +88,9 @@ private Property restrictByValue(Property property, String value) { var coercing = tryCoerce(property.name(), value); if (coercing != null) { return new Property.CoercingSubProperty(property, coercing, jsonLd); - } else if (property.name().equals("librissearch:workCategory")) { + } else if (property.name().equals(WORK_CATEGORY)) { // FIXME: Don't hardcode - return new Property.CoercingSubProperty(property, "librissearch:noneCategory", jsonLd); + return new Property.CoercingSubProperty(property, NONE_CATEGORY, jsonLd); } return property; } diff --git a/whelk-core/src/main/groovy/whelk/search2/Query.java b/whelk-core/src/main/groovy/whelk/search2/Query.java index 188dc0a52e..c3a049b3cf 100644 --- a/whelk-core/src/main/groovy/whelk/search2/Query.java +++ b/whelk-core/src/main/groovy/whelk/search2/Query.java @@ -70,11 +70,11 @@ public class Query { private ReducedQueryTree fullQueryTree; - private static final String WORK_CATEGORY = "librissearch:workCategory"; + static final String WORK_CATEGORY = "librissearch:workCategory"; private static final String FIND_CATEGORY = "librissearch:findCategory"; private static final String IDENTIFY_CATEGORY = "librissearch:identifyCategory"; - private static final String NONE_CATEGORY = "librissearch:noneCategory"; + static final String NONE_CATEGORY = "librissearch:noneCategory"; public enum SearchMode { SUGGEST,