Merge branch 'master' of https://github.com/OpenRefine/OpenRefine

.travis.yml

@@ -1,7 +1,17 @@
 language: java
 jdk:
   - oraclejdk8
-  - oraclejdk7
   - openjdk7
+
+env:
+  # encrypted Codacy key, see https://docs.travis-ci.com/user/encryption-keys/
+  - secure: "VmS4He99YlI6rdmw8Q25OZ9kUp11sRbt0W1QMBvA5lzNSmhN1Q1KtaMj9AGwpCZWcyGWri4AQxEmloARxACxQHXRmNE7ro2DESGw46RAocBAf+RfBxYTifIyUGu5TnSCQhz56SkgpyWpedZAZWyah9ZxgUMfet4KXFUfeiUgYQA="
   
-script: ./refine test
+before_install:
+  - wget -O ~/codacy-coverage-reporter-assembly-latest.jar https://github.com/codacy/codacy-coverage-reporter/releases/download/2.0.0/codacy-coverage-reporter-2.0.0-assembly.jar
+
+script: ./refine server_test
+
+after_success:
+  - java -cp ~/codacy-coverage-reporter-assembly-latest.jar com.codacy.CodacyCoverageReporter -l Java -r build/report/jacoco.xml
+

README.md

@@ -1,6 +1,6 @@
 # OpenRefine
 
-[![Build Status](https://travis-ci.org/OpenRefine/OpenRefine.png?branch=master)](https://travis-ci.org/OpenRefine/OpenRefine) [![Bountysource](https://www.bountysource.com/badge/tracker?tracker_id=32795)](https://www.bountysource.com/trackers/32795-open-refine?utm_source=32795&utm_medium=shield&utm_campaign=TRACKER_BADGE)
+[![Build Status](https://travis-ci.org/OpenRefine/OpenRefine.png?branch=master)](https://travis-ci.org/OpenRefine/OpenRefine) [![Codacy Badge](https://api.codacy.com/project/badge/Grade/210578308bba42c5922c767493e83cf4)](https://www.codacy.com/app/OpenRefine/OpenRefine) [![Bountysource](https://www.bountysource.com/badge/tracker?tracker_id=32795)](https://www.bountysource.com/trackers/32795-open-refine?utm_source=32795&utm_medium=shield&utm_campaign=TRACKER_BADGE)
 
 OpenRefine is a power tool that allows you to load data, understand it,
 clean it up, reconcile it, and augment it with data coming from

build.xml

@@ -6,7 +6,7 @@
     |
     +-->
     
-<project name="openrefine" default="build" basedir=".">
+<project name="openrefine" default="build" basedir="." xmlns:jacoco="antlib:org.jacoco.ant">
 
     <!-- Properties are immutable, so anything set in this file takes priority-->
     <property file="build.properties"/>
@@ -49,6 +49,9 @@
     <property name="server.tests.lib.dir" value="${server.tests.dir}/lib" />
     <property name="server.tests.src.dir" value="${server.tests.dir}/src" />
     <property name="server.tests.classes.dir" value="${server.tests.dir}/classes" />
+    <property name="server.tests.report.dir" value="${build.dir}/report"/>
+    <property name="server.tests.report.html.dir" value="${server.tests.report.dir}/html"/>
+    <property name="server.tests.report.xml.path" value="${server.tests.report.dir}/jacoco.xml"/>
     
     <property name="graphics.dir" value="${basedir}/graphics" />
     <property name="conf.dir" value="${basedir}/conf" />
@@ -171,13 +174,35 @@
     <target name="build_broker" depends="build_broker_core, build_broker_appengine"/>
 
     <taskdef resource="testngtasks" classpath="${server.tests.lib.dir}/testng-6.8.jar"/>
+    <taskdef uri="antlib:org.jacoco.ant" resource="org/jacoco/ant/antlib.xml">
+        <classpath path="${server.tests.lib.dir}/jacocoant.jar"/>
+    </taskdef>
+
     <mkdir dir="${build.dir}/server_tests"/>
     <target name="server_test" depends="build_tests">
-    	<testng verbose="2" haltOnFailure="true" workingdir="${build.dir}/server_tests"
+	<jacoco:coverage destfile="${server.tests.report.dir}/jacoco.exec">
-    		listener="org.testng.reporters.DotTestListener" excludedgroups="broken"
+            <testng verbose="2" haltOnFailure="true" workingdir="${build.dir}/server_tests"
-    		classpathref="tests.class.path">
+                listener="org.testng.reporters.DotTestListener" excludedgroups="broken"
-    	  <xmlfileset file="${server.tests.dir}/conf/tests.xml"/>
+                classpathref="tests.class.path">
-    	</testng>
+             <xmlfileset file="${server.tests.dir}/conf/tests.xml"/>
+            </testng>
+	</jacoco:coverage>
+        <jacoco:report>
+           <executiondata>
+		   <file file="${server.tests.report.dir}/jacoco.exec"/>
+           </executiondata>
+
+           <structure name="OpenRefine server">
+               <classfiles>
+		       <fileset dir="${webapp.classes.dir}"/>
+               </classfiles>
+                <sourcefiles encoding="UTF-8">
+	            <fileset dir="${server.src.dir}"/>
+                </sourcefiles>
+            </structure>
+	    <html destdir="${server.tests.report.html.dir}"/>
+	    <xml destfile="${server.tests.report.xml.path}"/>
+        </jacoco:report>
     </target>
 
     <!-- ================================================================== -->

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<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;
             }
         }

main/tests/server/src/com/google/refine/tests/model/KeyValueColumnizeTests.java

@@ -0,0 +1,173 @@
+/*
+
+Copyright 2010, Google Inc.
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are
+met:
+
+    * Redistributions of source code must retain the above copyright
+notice, this list of conditions and the following disclaimer.
+    * Redistributions in binary form must reproduce the above
+copyright notice, this list of conditions and the following disclaimer
+in the documentation and/or other materials provided with the
+distribution.
+    * Neither the name of Google Inc. nor the names of its
+contributors may be used to endorse or promote products derived from
+this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,           
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY           
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+*/
+
+package com.google.refine.tests.model;
+
+import static org.mockito.Mockito.mock;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.StringReader;
+import java.util.Properties;
+import java.util.List;
+import java.util.ArrayList;
+
+import org.json.JSONException;
+import org.json.JSONObject;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.BeforeTest;
+import org.testng.annotations.Test;
+
+import com.google.refine.ProjectManager;
+import com.google.refine.ProjectMetadata;
+import com.google.refine.RefineServlet;
+import com.google.refine.importers.SeparatorBasedImporter;
+import com.google.refine.importing.ImportingJob;
+import com.google.refine.importing.ImportingManager;
+import com.google.refine.io.FileProjectManager;
+import com.google.refine.model.AbstractOperation;
+import com.google.refine.model.ModelException;
+import com.google.refine.model.Project;
+import com.google.refine.process.Process;
+import com.google.refine.operations.cell.KeyValueColumnizeOperation;
+import com.google.refine.tests.RefineServletStub;
+import com.google.refine.tests.RefineTest;
+import com.google.refine.tests.util.TestUtils;
+
+
+public class KeyValueColumnizeTests extends RefineTest {
+    // dependencies
+    private RefineServlet servlet;
+    private Project project;
+    private ProjectMetadata pm;
+    private JSONObject options;
+    private ImportingJob job;
+    private SeparatorBasedImporter importer;
+
+
+    @Override
+    @BeforeTest
+    public void init() {
+        logger = LoggerFactory.getLogger(this.getClass());
+    }
+
+    @BeforeMethod
+    public void SetUp() throws JSONException, IOException, ModelException {
+	servlet = new RefineServletStub();
+        File dir = TestUtils.createTempDirectory("openrefine-test-workspace-dir");
+        FileProjectManager.initialize(dir);
+        project = new Project();
+        pm = new ProjectMetadata();
+        pm.setName("KeyValueColumnize test");
+        ProjectManager.singleton.registerProject(project, pm);
+        options = mock(JSONObject.class);
+
+	ImportingManager.initialize(servlet);
+        job = ImportingManager.createJob();
+	importer = new SeparatorBasedImporter(); 
+    }
+
+    @AfterMethod
+    public void TearDown() {
+	ImportingManager.disposeJob(job.id);
+	ProjectManager.singleton.deleteProject(project.id);
+	job = null;
+        project = null;
+	pm = null;
+        options = null;
+    }
+
+    /**
+     * Test to demonstrate the intended behaviour of the function, for issue #1214
+     * https://github.com/OpenRefine/OpenRefine/issues/1214
+     */
+
+    @Test
+    public void testKeyValueColumnize() throws Exception {
+	String csv = "Key,Value\n"
+		+ "merchant,Katie\n"
+		+ "fruit,apple\n"
+		+ "price,1.2\n"
+		+ "fruit,pear\n"
+		+ "price,1.5\n"
+		+ "merchant,John\n"
+		+ "fruit,banana\n"
+		+ "price,3.1\n";
+	prepareOptions(",", 20, 0, 0, 1, false, false);
+        List<Exception> exceptions = new ArrayList<Exception>();
+        importer.parseOneFile(project, pm, job, "filesource", new StringReader(csv), -1, options, exceptions);
+        project.update();
+        ProjectManager.singleton.registerProject(project, pm);
+
+	AbstractOperation op = new KeyValueColumnizeOperation(
+		"Key",
+		"Value",
+		null);
+        Process process = op.createProcess(project, new Properties());
+        process.performImmediate();
+
+	int merchantCol = project.columnModel.getColumnByName("merchant").getCellIndex();
+	int fruitCol = project.columnModel.getColumnByName("fruit").getCellIndex();
+	int priceCol = project.columnModel.getColumnByName("price").getCellIndex();
+	
+	Assert.assertEquals(project.rows.get(0).getCellValue(merchantCol), "Katie");
+	Assert.assertEquals(project.rows.get(1).getCellValue(merchantCol), null);
+	Assert.assertEquals(project.rows.get(2).getCellValue(merchantCol), "John");
+	Assert.assertEquals(project.rows.get(0).getCellValue(fruitCol), "apple");
+	Assert.assertEquals(project.rows.get(1).getCellValue(fruitCol), "pear");
+	Assert.assertEquals(project.rows.get(2).getCellValue(fruitCol), "banana");
+	Assert.assertEquals(project.rows.get(0).getCellValue(priceCol), "1.2");
+	Assert.assertEquals(project.rows.get(1).getCellValue(priceCol), "1.5");
+	Assert.assertEquals(project.rows.get(2).getCellValue(priceCol), "3.1");
+    }
+
+    private void prepareOptions(
+            String sep, int limit, int skip, int ignoreLines,
+            int headerLines, boolean guessValueType, boolean ignoreQuotes) {
+            
+            whenGetStringOption("separator", options, sep);
+            whenGetIntegerOption("limit", options, limit);
+            whenGetIntegerOption("skipDataLines", options, skip);
+            whenGetIntegerOption("ignoreLines", options, ignoreLines);
+            whenGetIntegerOption("headerLines", options, headerLines);
+            whenGetBooleanOption("guessCellValueTypes", options, guessValueType);
+            whenGetBooleanOption("processQuotes", options, !ignoreQuotes);
+            whenGetBooleanOption("storeBlankCellsAsNulls", options, true);
+        }
+
+
+}
+

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)));
+    }
+
 }