s434730/RandomSec
3
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix Recon id generation. (#4217)

Browse Source 
The previous method was prone to creating collisions when a lot of recon ids
were created around the same time.

Closes #3785.
This commit is contained in:
Antonin Delpeuch 2021-10-20 11:28:42 +02:00 committed by GitHub
parent 00e425fd9b
commit c71dee891c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 26 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
main/src/com/google/refine/model/Recon.java
View File

@ -39,6 +39,7 @@ import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Properties; import java.util.Properties;
import java.util.Random;
import com.fasterxml.jackson.annotation.JsonFilter; import com.fasterxml.jackson.annotation.JsonFilter;
import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonIgnore;
@ -61,6 +62,7 @@ public class Recon implements HasFields {
private static final String WIKIDATA_SCHEMA_SPACE = "http://www.wikidata.org/prop/direct/"; private static final String WIKIDATA_SCHEMA_SPACE = "http://www.wikidata.org/prop/direct/";
private static final String WIKIDATA_IDENTIFIER_SPACE = "http://www.wikidata.org/entity/"; private static final String WIKIDATA_IDENTIFIER_SPACE = "http://www.wikidata.org/entity/";
private static final Random idGenerator = new Random();
static public enum Judgment { static public enum Judgment {
@JsonProperty("none") @JsonProperty("none")
@ -151,7 +153,7 @@ public class Recon implements HasFields {
} }
public Recon(long judgmentHistoryEntry, String identifierSpace, String schemaSpace) { public Recon(long judgmentHistoryEntry, String identifierSpace, String schemaSpace) {
id = System.currentTimeMillis() * 1000000 + Math.round(Math.random() * 1000000); id = idGenerator.nextLong();
this.judgmentHistoryEntry = judgmentHistoryEntry; this.judgmentHistoryEntry = judgmentHistoryEntry;
this.identifierSpace = identifierSpace; this.identifierSpace = identifierSpace;
this.schemaSpace = schemaSpace; this.schemaSpace = schemaSpace;

23
main/tests/server/src/com/google/refine/model/ReconTests.java
View File

@ -26,6 +26,11 @@
******************************************************************************/ ******************************************************************************/
package com.google.refine.model; package com.google.refine.model;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.LongStream;
import org.testng.Assert;
import org.testng.annotations.Test; import org.testng.annotations.Test;
import com.google.refine.model.Recon.Judgment; import com.google.refine.model.Recon.Judgment;
@ -96,5 +101,23 @@ public class ReconTests {
r.judgment = Judgment.None; r.judgment = Judgment.None;
TestUtils.isSerializedTo(r, json); TestUtils.isSerializedTo(r, 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.
*/
@Test
public void randomIdGeneration() {
long numberOfSamples = 100000L;
String space = "http://some.url/";
long judgmentHistoryId = 1234L;
Set<Long> ids = LongStream.range(0L, numberOfSamples)
.mapToObj(i -> new Recon(judgmentHistoryId, space, space).id)
.collect(Collectors.toSet());
// make sure we generated as many ids as Recon objects (if ids.size() is smaller,
// then we have had some collisions)
Assert.assertEquals(ids.size(), numberOfSamples);
}
} }