Merge pull request #1682 from OpenRefine/issue1680

Fix snak grouping for qualifiers and references in Wikidata extension.

extensions/wikidata/src/org/openrefine/wikidata/schema/WbReferenceExpr.java

@@ -59,16 +59,15 @@ public class WbReferenceExpr implements WbExpression<Reference> {
     @Override
     public Reference evaluate(ExpressionContext ctxt)
             throws SkipSchemaExpressionException {
-        List<SnakGroup> snakGroups = new ArrayList<SnakGroup>();
+        List<Snak> snakList = new ArrayList<Snak>();
         for (WbSnakExpr expr : getSnaks()) {
-            List<Snak> snakList = new ArrayList<Snak>(1);
             try {
                 snakList.add(expr.evaluate(ctxt));
-                snakGroups.add(Datamodel.makeSnakGroup(snakList));
             } catch (SkipSchemaExpressionException e) {
                 continue;
             }
         }
+        List<SnakGroup> snakGroups = WbStatementExpr.groupSnaks(snakList);
         if (!snakGroups.isEmpty()) {
             return Datamodel.makeReference(snakGroups);
         } else {

extensions/wikidata/src/org/openrefine/wikidata/schema/WbStatementExpr.java

@@ -25,7 +25,10 @@ package org.openrefine.wikidata.schema;
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
 
 import org.jsoup.helper.Validate;
 import org.openrefine.wikidata.qa.QAWarning;
@@ -69,13 +72,22 @@ public class WbStatementExpr {
     }
 
     public static List<SnakGroup> groupSnaks(List<Snak> snaks) {
-        List<SnakGroup> snakGroups = new ArrayList<SnakGroup>();
+        Map<PropertyIdValue, List<Snak>> snakGroups = new HashMap<>();
+        List<PropertyIdValue> propertyOrder = new ArrayList<PropertyIdValue>();
         for (Snak snak : snaks) {
-            List<Snak> singleton = new ArrayList<Snak>();
+            List<Snak> existingSnaks = snakGroups.get(snak.getPropertyId());
-            singleton.add(snak);
+            if(existingSnaks == null) {
-            snakGroups.add(Datamodel.makeSnakGroup(singleton));
+                existingSnaks = new ArrayList<Snak>();
+                snakGroups.put(snak.getPropertyId(), existingSnaks);
+                propertyOrder.add(snak.getPropertyId());
+            }
+            if (!existingSnaks.contains(snak)) {
+                existingSnaks.add(snak);
+            }
         }
-        return snakGroups;
+        return propertyOrder.stream()
+                .map(pid -> Datamodel.makeSnakGroup(snakGroups.get(pid)))
+                .collect(Collectors.toList());
     }
 
     public Statement evaluate(ExpressionContext ctxt, ItemIdValue subject, PropertyIdValue propertyId)

extensions/wikidata/tests/src/org/openrefine/wikidata/schema/WbStatementExprTest.java

@@ -55,6 +55,11 @@ public class WbStatementExprTest extends WbExpressionTest<Statement> {
     private WbLocationVariable mainValueExpr = new WbLocationVariable("column C");
     public WbStatementExpr statementExpr = new WbStatementExpr(mainValueExpr, Collections.singletonList(qualifierExpr),
             Collections.singletonList(refExpr));
+    private WbSnakExpr constantQualifierExpr = new WbSnakExpr(new WbPropConstant("P897", "point in time", "time"),
+            new WbDateConstant("2018-04-05"));
+    public WbStatementExpr statementWithConstantExpr = new WbStatementExpr(mainValueExpr,
+            Arrays.asList(qualifierExpr, constantQualifierExpr),
+            Collections.singletonList(refExpr));
 
     public ItemIdValue subject = Datamodel.makeWikidataItemIdValue("Q23");
     private PropertyIdValue property = Datamodel.makeWikidataPropertyIdValue("P908");
@@ -63,12 +68,18 @@ public class WbStatementExprTest extends WbExpressionTest<Statement> {
                     Datamodel.makeWikidataItemIdValue("Q3434"))))));
     private Snak qualifier = Datamodel.makeValueSnak(Datamodel.makeWikidataPropertyIdValue("P897"),
             Datamodel.makeTimeValue(2010, (byte) 7, (byte) 23, (byte) 0, (byte) 0, (byte) 0, (byte) 11, 0, 0, 0, TimeValue.CM_GREGORIAN_PRO));
+    private Snak constantQualifier = Datamodel.makeValueSnak(Datamodel.makeWikidataPropertyIdValue("P897"),
+            Datamodel.makeTimeValue(2018, (byte) 4, (byte) 5, (byte) 0, (byte) 0, (byte) 0, (byte) 11, 0, 0, 0, TimeValue.CM_GREGORIAN_PRO));
     private Snak mainsnak = Datamodel.makeValueSnak(property, Datamodel.makeGlobeCoordinatesValue(3.898, 4.389,
             WbLocationConstant.defaultPrecision, GlobeCoordinatesValue.GLOBE_EARTH));
     private Claim fullClaim = Datamodel.makeClaim(subject, mainsnak,
             Collections.singletonList(Datamodel.makeSnakGroup(Collections.singletonList(qualifier))));
     public Statement fullStatement = Datamodel.makeStatement(fullClaim, Collections.singletonList(reference),
             StatementRank.NORMAL, "");
+    public Claim claimWithConstant = Datamodel.makeClaim(subject, mainsnak,
+            Collections.singletonList(Datamodel.makeSnakGroup(Arrays.asList(qualifier, constantQualifier))));
+    public Statement statementWithConstant = Datamodel.makeStatement(claimWithConstant, Collections.singletonList(reference),
+            StatementRank.NORMAL, "");
 
     class Wrapper implements WbExpression<Statement> {
 
@@ -105,6 +116,12 @@ public class WbStatementExprTest extends WbExpressionTest<Statement> {
         setRow(recon("Q3434"), "2010-07-23", "3.898,4.389");
         evaluatesTo(fullStatement, new Wrapper(statementExpr));
     }
+    
+    @Test
+    public void testEvaluateWithConstant() {
+        setRow(recon("Q3434"), "2010-07-23", "3.898,4.389");
+        evaluatesTo(statementWithConstant, new Wrapper(statementWithConstantExpr));
+    }
 
     @Test
     public void testEvaluateWithoutReference() {