Merge pull request #1222 from OpenRefine/issue1219
Ensure null values are not cached in URL fetching operation.
This commit is contained in:
commit
820b6f395a
@ -202,7 +202,7 @@ public class ColumnAdditionByFetchingURLsOperation extends EngineDependentOperat
|
||||
.expireAfterWrite(10, TimeUnit.MINUTES)
|
||||
.build(
|
||||
new CacheLoader<String, Serializable>() {
|
||||
public Serializable load(String urlString) {
|
||||
public Serializable load(String urlString) throws Exception {
|
||||
Serializable result = fetch(urlString);
|
||||
try {
|
||||
// Always sleep for the delay, no matter how long the
|
||||
@ -214,8 +214,13 @@ public class ColumnAdditionByFetchingURLsOperation extends EngineDependentOperat
|
||||
Thread.sleep(_delay);
|
||||
}
|
||||
} catch (InterruptedException e) {
|
||||
return null;
|
||||
result = null;
|
||||
}
|
||||
|
||||
if (result == null) {
|
||||
// the load method should not return any null value
|
||||
throw new Exception("null result returned by fetch");
|
||||
}
|
||||
return result;
|
||||
}
|
||||
});
|
||||
@ -304,7 +309,7 @@ public class ColumnAdditionByFetchingURLsOperation extends EngineDependentOperat
|
||||
Serializable cachedFetch(String urlString) {
|
||||
try {
|
||||
return _urlCache.get(urlString);
|
||||
} catch(ExecutionException e) {
|
||||
} catch(Exception e) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
@ -54,6 +54,7 @@ import com.google.refine.browsing.Engine;
|
||||
import com.google.refine.browsing.RowVisitor;
|
||||
import com.google.refine.grel.Function;
|
||||
import com.google.refine.io.FileProjectManager;
|
||||
import com.google.refine.expr.ExpressionUtils;
|
||||
import com.google.refine.model.Cell;
|
||||
import com.google.refine.model.Column;
|
||||
import com.google.refine.model.ModelException;
|
||||
@ -160,5 +161,46 @@ public class UrlFetchingTests extends RefineTest {
|
||||
Assert.assertEquals(project.rows.get(i).getCellValue(1), ref_val);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Fetch invalid URLs
|
||||
* https://github.com/OpenRefine/OpenRefine/issues/1219
|
||||
*/
|
||||
@Test
|
||||
public void testInvalidUrl() throws Exception {
|
||||
Row row0 = new Row(2);
|
||||
row0.setCell(0, new Cell("auinrestrsc", null)); // malformed -> null
|
||||
project.rows.add(row0);
|
||||
Row row1 = new Row(2);
|
||||
row1.setCell(0, new Cell("https://www.random.org/integers/?num=1&min=1&max=100&col=1&base=10&format=plain", null)); // fine
|
||||
project.rows.add(row1);
|
||||
Row row2 = new Row(2);
|
||||
row2.setCell(0, new Cell("http://anursiebcuiesldcresturce.detur/anusclbc", null)); // well-formed but invalid
|
||||
project.rows.add(row2);
|
||||
EngineDependentOperation op = new ColumnAdditionByFetchingURLsOperation(engine_config,
|
||||
"fruits",
|
||||
"value",
|
||||
OnError.StoreError,
|
||||
"junk",
|
||||
1,
|
||||
50,
|
||||
true);
|
||||
ProcessManager pm = project.getProcessManager();
|
||||
Process process = op.createProcess(project, options);
|
||||
process.startPerforming(pm);
|
||||
Assert.assertTrue(process.isRunning());
|
||||
try {
|
||||
Thread.sleep(5000);
|
||||
} catch (InterruptedException e) {
|
||||
Assert.fail("Test interrupted");
|
||||
}
|
||||
Assert.assertFalse(process.isRunning());
|
||||
|
||||
int newCol = project.columnModel.getColumnByName("junk").getCellIndex();
|
||||
// Inspect rows
|
||||
Assert.assertEquals(project.rows.get(0).getCellValue(newCol), null);
|
||||
Assert.assertTrue(project.rows.get(1).getCellValue(newCol) != null);
|
||||
Assert.assertTrue(ExpressionUtils.isError(project.rows.get(2).getCellValue(newCol)));
|
||||
}
|
||||
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user