Repository: OpenRefine/OpenRefine (java)

Issue:
Impossible to load a project again after manually matching reconciled cells with errors
In some circumstances it is possible that loading a previously saved project fails in 3.8-beta1.

### To Reproduce
Steps to reproduce the behavior:
1. First, reconcile a column to a reconciliation service that returns errors
2. Then, manually match some cell with an error to an entity
3. Close OpenRefine
4. Start OpenRefine and open the project again

### Current Results

The project does not load and logs show:

```
com.fasterxml.jackson.databind.exc.ValueInstantiationException: Cannot construct instance of `com.google.refine.model.Recon`, problem: there is a match hence no error
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 445]
	at com.fasterxml.jackson.databind.exc.ValueInstantiationException.from(ValueInstantiationException.java:47)
	at com.fasterxml.jackson.databind.DeserializationContext.instantiationException(DeserializationContext.java:2014)
	at com.fasterxml.jackson.databind.deser.std.StdValueInstantiator.wrapAsJsonMappingException(StdValueInstantiator.java:598)
	at com.fasterxml.jackson.databind.deser.std.StdValueInstantiator.rewrapCtorProblem(StdValueInstantiator.java:621)
	at com.fasterxml.jackson.databind.deser.std.StdValueInstantiator.createFromObjectWith(StdValueInstantiator.java:293)
	at com.fasterxml.jackson.databind.deser.ValueInstantiator.createFromObjectWith(ValueInstantiator.java:301)
	at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.build(PropertyBasedCreator.java:202)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:445)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1493)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:348)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:185)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:342)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4899)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3846)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3814)
	at com.google.refine.model.Recon.loadStreaming(Recon.java:392)
	at com.google.refine.util.Pool.load(Pool.java:151)
	at com.google.refine.util.Pool.load(Pool.java:118)
	at com.google.refine.io.ProjectUtilities.loadFromFile(ProjectUtilities.java:153)
	at com.google.refine.io.ProjectUtilities.load(ProjectUtilities.java:118)
	at com.google.refine.io.FileProjectManager.loadProject(FileProjectManager.java:245)
	at com.google.refine.ProjectManager.getProject(ProjectManager.java:555)
```

### Expected Behavior

The project should be loaded again in the state it was left in.

### Additional context

This is due to the checks we have introduced in the deserialization of Recon objects, which assume that a recon object cannot both be matched to an entity and contain an error.

When matching a cell with a reconciliation error to an entity, we do not currently discard the error it contains, leading to this inconsistency.

I would argue that we should in fact keep the error, so that if we unmatch the cell, we get back to the original state of the cell after reconciliation, containing the original error.
I would propose to change the "invariant" that we try to enforce in the Recon class, to be that a Recon object cannot contain an error and a non-empty list of candidates at the same time.

As a side note, this bug would not have been allowed to happen if the Recon class had been immutable, making sure that any "changes" require going through a new constructor call which is allowed to check consistency of all fields. As an initial remediation we could already use encapsulation with setters to modify fields, making it possible to control which changes are allowed.

cc @ayushrai206 


Pull request:
Fix recon deserialization error for matched cells
Closes #6464.


Patch:
diff --git a/extensions/gdata/pom.xml b/extensions/gdata/pom.xml
index 857e0538a..ea722b947 100644
--- a/extensions/gdata/pom.xml
+++ b/extensions/gdata/pom.xml
@@ -89,7 +89,7 @@
     <dependency>
       <groupId>com.google.apis</groupId>
       <artifactId>google-api-services-sheets</artifactId>
-      <version>v4-rev20240312-2.0.0</version>
+      <version>v4-rev20240319-2.0.0</version>
     </dependency>
     <dependency>
       <groupId>com.fasterxml.jackson.core</groupId>
diff --git a/extensions/wikibase/module/scripts/dialogs/wikibase-dialog.html b/extensions/wikibase/module/scripts/dialogs/wikibase-dialog.html
index 80a26a324..3c32eb052 100644
--- a/extensions/wikibase/module/scripts/dialogs/wikibase-dialog.html
+++ b/extensions/wikibase/module/scripts/dialogs/wikibase-dialog.html
@@ -14,8 +14,8 @@
       <a class="button" bind="discoverManifestsButton" href="https://github.com/OpenRefine/wikibase-manifests" target="_blank"></a>
     </div>
     <div class="dialog-footer-bg">
+      <button class="button button-primary" bind="okButton"></button>
       <button class="button" bind="cancelButton"></button>
-      <button class="button" bind="okButton"></button>
     </div>
   </div>
 </div>
diff --git a/main/src/com/google/refine/model/Recon.java b/main/src/com/google/refine/model/Recon.java
index 54fc9fe62..4875bff38 100644
--- a/main/src/com/google/refine/model/Recon.java
+++ b/main/src/com/google/refine/model/Recon.java
@@ -214,6 +214,7 @@ public class Recon implements HasFields {
             candidates = new ArrayList<ReconCandidate>(3);
         }
         candidates.add(candidate);
+        error = null;
     }
 
     @JsonIgnore
@@ -412,11 +413,11 @@ public class Recon implements HasFields {
         this.judgment = judgment != null ? judgment : Judgment.None;
         this.match = match;
         this.error = error;
-        if (error != null && match != null) {
-            throw new IllegalArgumentException("there is a match hence no error");
-        }
         this.features = features != null ? features : new Object[Feature_max];
         this.candidates = candidates != null ? candidates : new ArrayList<>();
+        if (error != null && !this.candidates.isEmpty()) {
+            throw new IllegalArgumentException("Recon deserialization: inconsistent state - error & candidates not allowed together");
+        }
         this.service = service != null ? service : "unknown";
         this.identifierSpace = identifierSpace;
         this.schemaSpace = schemaSpace;
diff --git a/main/webapp/modules/core/scripts/dialogs/column-reordering-dialog.html b/main/webapp/modules/core/scripts/dialogs/column-reordering-dialog.html
index 4b3e7819f..cfa397ad7 100644
--- a/main/webapp/modules/core/scripts/dialogs/column-reordering-dialog.html
+++ b/main/webapp/modules/core/scripts/dialogs/column-reordering-dialog.html
@@ -15,7 +15,7 @@
     </tr>
   </table></div></div>
   <div class="dialog-footer" bind="dialogFooter">
-    <button class="button" bind="okButton"></button>
+    <button class="button button-primary" bind="okButton"></button>
     <button class="button" bind="cancelButton"></button>
   </div>
 </div>
diff --git a/main/webapp/modules/core/scripts/dialogs/common-transform-dialog.html b/main/webapp/modules/core/scripts/dialogs/common-transform-dialog.html
index cf431ce05..9260cdfe4 100644
--- a/main/webapp/modules/core/scripts/dialogs/common-transform-dialog.html
+++ b/main/webapp/modules/core/scripts/dialogs/common-transform-dialog.html
@@ -8,7 +8,7 @@
       </div>
     </div>
     <div  class="all-column-transform-dialog-container dialog-footer" bind="dialogFooter">
-      <button class="button"  bind="okButton"></button>
+      <button class="button button-primary"  bind="okButton"></button>
       <button class="button"  bind="cancelButton"></button>
     </div>
 </div>
diff --git a/main/webapp/modules/core/scripts/dialogs/expression-column-dialog.html b/main/webapp/modules/core/scripts/dialogs/expression-column-dialog.html
index 2104a85dd..7b33e0999 100644
--- a/main/webapp/modules/core/scripts/dialogs/expression-column-dialog.html
+++ b/main/webapp/modules/core/scripts/dialogs/expression-column-dialog.html
@@ -20,7 +20,7 @@
     </div>
   </div>
   <div class="dialog-footer" bind="dialogFooter">
-    <button class="button" bind="okButton"></button>
+    <button class="button button-primary" bind="okButton"></button>
     <button class="button" bind="cancelButton"></button>
   </div>
 </div>
diff --git a/main/webapp/modules/core/scripts/views/data-table/add-column-by-fetching-urls-dialog.html b/main/webapp/modules/core/scripts/views/data-table/add-column-by-fetching-urls-dialog.html
index 0655bc74a..ceeae19fe 100644
--- a/main/webapp/modules/core/scripts/views/data-table/add-column-by-fetching-urls-dialog.html
+++ b/main/webapp/modules/core/scripts/views/data-table/add-column-by-fetching-urls-dialog.html
@@ -34,7 +34,7 @@
       </table></div>
     </div>
     <div class="dialog-footer" bind="dialogFooter">
-      <button class="button" type="submit" bind="okButton"></button>
+      <button class="button button-primary" type="submit" bind="okButton"></button>
       <button class="button" bind="cancelButton"></button>
     </div>
     </form>
diff --git a/main/webapp/modules/core/scripts/views/data-table/add-column-by-reconciliation.html b/main/webapp/modules/core/scripts/views/data-table/add-column-by-reconciliation.html
index ecdde8301..60ad2bfc6 100644
--- a/main/webapp/modules/core/scripts/views/data-table/add-column-by-reconciliation.html
+++ b/main/webapp/modules/core/scripts/views/data-table/add-column-by-reconciliation.html
@@ -23,7 +23,7 @@
     </table></div>
   </div>
   <div class="dialog-footer" bind="dialogFooter">
-    <button class="button" bind="okButton"></button>
+    <button class="button button-primary" bind="okButton"></button>
     <button class="button" bind="cancelButton"></button>
   </div>
 </div>
diff --git a/main/webapp/modules/core/scripts/views/data-table/add-column-dialog.html b/main/webapp/modules/core/scripts/views/data-table/add-column-dialog.html
index 850aa13c1..cc26f8f7b 100644
--- a/main/webapp/modules/core/scripts/views/data-table/add-column-dialog.html
+++ b/main/webapp/modules/core/scripts/views/data-table/add-column-dialog.html
@@ -22,7 +22,7 @@
       </table></div>
     </div>
     <div class="dialog-footer" bind="dialogFooter">
-      <button class="button" type="submit" bind="okButton"></button>
+      <button class="button button-primary" type="submit" bind="okButton"></button>
       <button class="button" bind="cancelButton"></button>
     </div>
   </form>
diff --git a/main/webapp/modules/core/scripts/views/data-table/add-q-column-dialog.html b/main/webapp/modules/core/scripts/views/data-table/add-q-column-dialog.html
index 2b9dc48ca..f937133f3 100644
--- a/main/webapp/modules/core/scripts/views/data-table/add-q-column-dialog.html
+++ b/main/webapp/modules/core/scripts/views/data-table/add-q-column-dialog.html
@@ -11,7 +11,7 @@
       </table></div>
     </div>
     <div class="dialog-footer" bind="dialogFooter">
-      <button type="submit" class="button" bind="okButton"></button>
+      <button type="submit" class="button button-primary" bind="okButton"></button>
       <button class="button" bind="cancelButton"></button>
     </div>
   </div>
diff --git a/main/webapp/modules/core/scripts/views/data-table/cell-editor.html b/main/webapp/modules/core/scripts/views/data-table/cell-editor.html
index 3b47d3b35..36c76c3e0 100644
--- a/main/webapp/modules/core/scripts/views/data-table/cell-editor.html
+++ b/main/webapp/modules/core/scripts/views/data-table/cell-editor.html
@@ -14,7 +14,7 @@
 
 <div id="data-table-cell-editor-actions">
   <div class="data-table-cell-editor-action">
-    <button class="button" bind="okButton"></button>
+    <button class="button button-primary" bind="okButton"></button>
     <div class="data-table-cell-editor-key" bind="or_views_enter"></div>
   </div>
   <div class="data-table-cell-editor-action">
diff --git a/main/webapp/modules/core/scripts/views/data-table/column-join.html b/main/webapp/modules/core/scripts/views/data-table/column-join.html
index 275bb7231..dce2a4844 100644
--- a/main/webapp/modules/core/scripts/views/data-table/column-join.html
+++ b/main/webapp/modules/core/scripts/views/data-table/column-join.html
@@ -58,7 +58,7 @@
      </div>
   </div>
   <div class="dialog-footer" bind="dialogFooter">
-    <button class="button" bind="okButton"></button>
+    <button class="button button-primary" bind="okButton"></button>
     <button class="button" bind="cancelButton"></button>
   </div>
 </div>
\ No newline at end of file
diff --git a/main/webapp/modules/core/scripts/views/data-table/copy-recon-across-columns-dialog.html b/main/webapp/modules/core/scripts/views/data-table/copy-recon-across-columns-dialog.html
index 5bbc0878e..777589ec8 100644
--- a/main/webapp/modules/core/scripts/views/data-table/copy-recon-across-columns-dialog.html
+++ b/main/webapp/modules/core/scripts/views/data-table/copy-recon-across-columns-dialog.html
@@ -31,7 +31,7 @@
       </table></div>
     </div>
     <div class="dialog-footer" bind="dialogFooter">
-      <button class="button" bind="okButton"></button>
+      <button class="button button-primary" bind="okButton"></button>
       <button class="button" bind="cancelButton"></button>
     </div>
   </div>
diff --git a/main/webapp/modules/core/scripts/views/data-table/key-value-columnize.html b/main/webapp/modules/core/scripts/views/data-table/key-value-columnize.html
index ae3bcc7bd..6280eb4bc 100644
--- a/main/webapp/modules/core/scripts/views/data-table/key-value-columnize.html
+++ b/main/webapp/modules/core/scripts/views/data-table/key-value-columnize.html
@@ -16,7 +16,7 @@
       </table></div>
     </div>
     <div class="dialog-footer" bind="dialogFooter">
-      <button class="button" bind="okButton"></button>
+      <button class="button button-primary" bind="okButton"></button>
       <button class="button" bind="cancelButton"></button>
     </div>
   </div>
diff --git a/main/webapp/modules/core/scripts/views/data-table/rename-column.html b/main/webapp/modules/core/scripts/views/data-table/rename-column.html
index 5dbf32b68..c635671bb 100644
--- a/main/webapp/modules/core/scripts/views/data-table/rename-column.html
+++ b/main/webapp/modules/core/scripts/views/data-table/rename-column.html
@@ -10,7 +10,7 @@
                 </div>
             </div>
             <div class="dialog-footer" bind="dialogFooter">
-                <button type="submit" class="button" bind="okButton"/>
+                <button type="submit" class="button button-primary" bind="okButton"/>
                 <button class="button" bind="cancelButton"></button>
             </div>
         </div>
diff --git a/main/webapp/modules/core/scripts/views/data-table/replace-dialog.html b/main/webapp/modules/core/scripts/views/data-table/replace-dialog.html
index 43d3e0247..4cba5c015 100644
--- a/main/webapp/modules/core/scripts/views/data-table/replace-dialog.html
+++ b/main/webapp/modules/core/scripts/views/data-table/replace-dialog.html
@@ -53,7 +53,7 @@
       </div>
     </div>
     <div class="dialog-footer" bind="dialogFooter">
-      <button class="button" bind="okButton"></button>
+      <button class="button button-primary" bind="okButton"></button>
       <button class="button" bind="cancelButton"></button>
     </div>
   </div>
diff --git a/main/webapp/modules/core/scripts/views/data-table/sorting-criterion-dialog.html b/main/webapp/modules/core/scripts/views/data-table/sorting-criterion-dialog.html
index 831af50b9..523f293a6 100644
--- a/main/webapp/modules/core/scripts/views/data-table/sorting-criterion-dialog.html
+++ b/main/webapp/modules/core/scripts/views/data-table/sorting-criterion-dialog.html
@@ -47,7 +47,7 @@
       </table></div>
     </div>
     <div class="dialog-footer" bind="dialogFooter">
-      <button class="button" bind="okButton"></button>
+      <button class="button button-primary" bind="okButton"></button>
       <button class="button" bind="cancelButton"></button>
     </div>
   </div>
diff --git a/main/webapp/modules/core/scripts/views/data-table/split-column-dialog.html b/main/webapp/modules/core/scripts/views/data-table/split-column-dialog.html
index cdcaaf7b6..46f9ef6c0 100644
--- a/main/webapp/modules/core/scripts/views/data-table/split-column-dialog.html
+++ b/main/webapp/modules/core/scripts/views/data-table/split-column-dialog.html
@@ -52,7 +52,7 @@
       </table></div>
     </div>
     <div class="dialog-footer" bind="dialogFooter">
-      <button class="button" bind="okButton"></button>
+      <button class="button button-primary" bind="okButton"></button>
       <button class="button" bind="cancelButton"></button>
     </div>
   </div>
diff --git a/main/webapp/modules/core/scripts/views/data-table/split-multi-valued-cells-dialog.html b/main/webapp/modules/core/scripts/views/data-table/split-multi-valued-cells-dialog.html
index f544d3bd5..36767bc15 100644
--- a/main/webapp/modules/core/scripts/views/data-table/split-multi-valued-cells-dialog.html
+++ b/main/webapp/modules/core/scripts/views/data-table/split-multi-valued-cells-dialog.html
@@ -55,7 +55,7 @@
       </table></div>
     </div>
     <div class="dialog-footer" bind="dialogFooter">
-      <button class="button" bind="okButton"></button>
+      <button class="button button-primary" bind="okButton"></button>
       <button class="button" bind="cancelButton"></button>
     </div>
   </div>
diff --git a/main/webapp/modules/core/scripts/views/data-table/text-transform-dialog.html b/main/webapp/modules/core/scripts/views/data-table/text-transform-dialog.html
index 87cf5bfdd..9c27e8ebf 100644
--- a/main/webapp/modules/core/scripts/views/data-table/text-transform-dialog.html
+++ b/main/webapp/modules/core/scripts/views/data-table/text-transform-dialog.html
@@ -21,7 +21,7 @@
     </div>
   </div>
   <div class="dialog-footer" bind="dialogFooter">
-    <button class="button" bind="okButton"></button>
+    <button class="button button-primary" bind="okButton"></button>
     <button class="button" bind="cancelButton"></button>
   </div>
   </div>
diff --git a/main/webapp/modules/core/scripts/views/data-table/transpose-columns-into-rows.html b/main/webapp/modules/core/scripts/views/data-table/transpose-columns-into-rows.html
index 0e1514a13..56f23b911 100644
--- a/main/webapp/modules/core/scripts/views/data-table/transpose-columns-into-rows.html
+++ b/main/webapp/modules/core/scripts/views/data-table/transpose-columns-into-rows.html
@@ -62,7 +62,7 @@
       </table></div>
     </div>
     <div class="dialog-footer" bind="dialogFooter">
-      <button class="button" bind="okButton"></button>
+      <button class="button button-primary" bind="okButton"></button>
       <button class="button" bind="cancelButton"></button>
     </div>
   </div>
diff --git a/main/webapp/modules/core/scripts/views/data-table/warn-of-pending-sort.html b/main/webapp/modules/core/scripts/views/data-table/warn-of-pending-sort.html
index 5cb4a77fe..f80aac8f1 100644
--- a/main/webapp/modules/core/scripts/views/data-table/warn-of-pending-sort.html
+++ b/main/webapp/modules/core/scripts/views/data-table/warn-of-pending-sort.html
@@ -10,7 +10,7 @@
             </div>
         </div>
         <div class="dialog-footer" bind="dialogFooter">
-            <button class="button" bind="okButton"></button>
+            <button class="button button-primary" bind="okButton"></button>
             <button class="button" bind="cancelButton"></button>
         </div>
     </div>
diff --git a/main/webapp/modules/core/styles/reconciliation/recon-dialog.css b/main/webapp/modules/core/styles/reconciliation/recon-dialog.css
index e035a75ea..24dec397e 100644
--- a/main/webapp/modules/core/styles/reconciliation/recon-dialog.css
+++ b/main/webapp/modules/core/styles/reconciliation/recon-dialog.css
@@ -104,7 +104,6 @@ a.recon-dialog-service-selector-remove {
 }
 .dialog-footer {
   display: flex;
-  justify-content: space-between;
 }
 
 .button-left {
diff --git a/pom.xml b/pom.xml
index 656dd1aaa..3c3637880 100644
--- a/pom.xml
+++ b/pom.xml
@@ -333,7 +333,7 @@
         <!-- Signature -->
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-gpg-plugin</artifactId>
-        <version>3.2.1</version>
+        <version>3.2.2</version>
         <executions>
             <execution>
                 <id>sign-artifacts</id>


Test patch:
diff --git a/main/tests/server/src/com/google/refine/model/ReconTests.java b/main/tests/server/src/com/google/refine/model/ReconTests.java
index 9d1c1ad5c..3e038494b 100644
--- a/main/tests/server/src/com/google/refine/model/ReconTests.java
+++ b/main/tests/server/src/com/google/refine/model/ReconTests.java
@@ -34,6 +34,7 @@ import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.LongStream;
 
+import com.fasterxml.jackson.databind.exc.ValueInstantiationException;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
@@ -123,6 +124,45 @@ public class ReconTests {
         TestUtils.isSerializedTo(r, json);
     }
 
+    @Test
+    public void testsErrorWithMatchSerialization() throws Exception {
+        String json = "{\"id\":1533651559492945033,"
+                + "\"service\":\"https://tools.wmflabs.org/openrefine-wikidata/en/api\","
+                + "\"identifierSpace\":\"http://www.wikidata.org/entity/\","
+                + "\"schemaSpace\":\"http://www.wikidata.org/prop/direct/\","
+                + "\"j\":\"matched\","
+                + "\"e\":\"fictional error message\","
+                + "\"m\":{"
+                + "   \"id\":\"Q2892284\","
+                + "   \"name\":\"Baylor College of Medicine\","
+                + "   \"score\":98.57142857142858,"
+                + "   \"types\":[\"Q16917\",\"Q23002054\",\"Q494230\"]"
+                + "}"
+                + "}";
+        Recon r = Recon.loadStreaming(json);
+        Assert.assertEquals(r.error, "fictional error message");
+        Assert.assertEquals(r.match.id, "Q2892284");
+        TestUtils.isSerializedTo(r, json);
+    }
+
+    @Test
+    public void testsErrorWithCandidatesDeserialization() throws Exception {
+        String json = "{\"id\":1533651559492945033,"
+                + "\"service\":\"https://tools.wmflabs.org/openrefine-wikidata/en/api\","
+                + "\"identifierSpace\":\"http://www.wikidata.org/entity/\","
+                + "\"schemaSpace\":\"http://www.wikidata.org/prop/direct/\","
+                + "\"j\":\"none\","
+                + "\"e\":\"fictional error message\","
+                + "\"c\":[{"
+                + "   \"id\":\"Q2892284\","
+                + "   \"name\":\"Baylor College of Medicine\","
+                + "   \"score\":98.57142857142858,"
+                + "   \"types\":[\"Q16917\",\"Q23002054\",\"Q494230\"]"
+                + "}]"
+                + "}";
+        Assert.assertThrows(ValueInstantiationException.class, () -> Recon.loadStreaming(json));
+    }
+
     /**
      * Test for issue https://github.com/OpenRefine/OpenRefine/issues/3785. Generating many recon objects within a short
      * amount of time leads to collisions in id generation.