diff --git a/main/src/com/google/refine/operations/column/ColumnAdditionByFetchingURLsOperation.java b/main/src/com/google/refine/operations/column/ColumnAdditionByFetchingURLsOperation.java index f64f98c58..f08f83887 100644 --- a/main/src/com/google/refine/operations/column/ColumnAdditionByFetchingURLsOperation.java +++ b/main/src/com/google/refine/operations/column/ColumnAdditionByFetchingURLsOperation.java @@ -202,7 +202,7 @@ public class ColumnAdditionByFetchingURLsOperation extends EngineDependentOperat .expireAfterWrite(10, TimeUnit.MINUTES) .build( new CacheLoader() { - 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; } } diff --git a/main/tests/server/src/com/google/refine/tests/model/UrlFetchingTests.java b/main/tests/server/src/com/google/refine/tests/model/UrlFetchingTests.java index 10419225f..3b84a5363 100644 --- a/main/tests/server/src/com/google/refine/tests/model/UrlFetchingTests.java +++ b/main/tests/server/src/com/google/refine/tests/model/UrlFetchingTests.java @@ -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))); + } + }