Source code cleanup (imports organised, httpclients closed, worker code moved)

src/main/java/org/genesys2/server/service/impl/ContentServiceImpl.java

@@ -16,6 +16,13 @@
 
 package org.genesys2.server.service.impl;
 
+import java.io.StringWriter;
+import java.io.Writer;
+import java.util.Calendar;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -41,13 +48,6 @@ import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
-import java.io.StringWriter;
-import java.io.Writer;
-import java.util.Calendar;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
-
 @Service
 @Transactional(readOnly = true)
 public class ContentServiceImpl implements ContentService {

src/main/java/org/genesys2/server/service/impl/EasySMTAConnector.java

@@ -31,12 +31,13 @@ import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.NameValuePair;
 import org.apache.http.client.ClientProtocolException;
-import org.apache.http.client.HttpClient;
 import org.apache.http.client.entity.UrlEncodedFormEntity;
 import org.apache.http.client.methods.HttpPost;
-import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.message.BasicNameValuePair;
 import org.genesys2.server.service.EasySMTA;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.annotation.Profile;
 import org.springframework.stereotype.Component;
@@ -49,6 +50,9 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 public class EasySMTAConnector implements EasySMTA {
 	private static final Log LOG = LogFactory.getLog(EasySMTAConnector.class);
 
+	@Autowired
+	private HttpClientBuilder httpClientBuilder;
+
 	@Value("${itpgrfa.easysmta.url}")
 	private String serverUrl;
 
@@ -60,8 +64,7 @@ public class EasySMTAConnector implements EasySMTA {
 
 	@Override
 	public EasySMTA.EasySMTAUserData getUserData(String emailAddress) throws EasySMTAException {
-		final HttpClient httpclient = new DefaultHttpClient();
-
+		
 		LOG.info("Checking EasySMTA at " + serverUrl + " for email: " + emailAddress);
 		final HttpPost httpPost = new HttpPost(serverUrl);
 
@@ -80,6 +83,7 @@ public class EasySMTAConnector implements EasySMTA {
 
 		final BufferedReader br = null;
 
+		final CloseableHttpClient httpclient = httpClientBuilder.build();
 		try {
 			LOG.info("Using " + serverUsername + " as Easy-SMTA username.");
 			response = httpclient.execute(httpPost);
@@ -118,6 +122,7 @@ public class EasySMTAConnector implements EasySMTA {
 		} finally {
 			IOUtils.closeQuietly(br);
 			httpPost.releaseConnection();
+			IOUtils.closeQuietly(httpclient);
 			LOG.debug("EasySMTA streams closed.");
 		}
 

src/main/java/org/genesys2/server/service/impl/GeoServiceImpl.java

@@ -28,9 +28,6 @@ import java.util.Set;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.genesys2.geo.sources.CountryInfo;
-import org.genesys2.geo.sources.DavrosCountrySource;
-import org.genesys2.geo.sources.GeoNamesCountrySource;
 import org.genesys2.server.model.impl.Country;
 import org.genesys2.server.model.impl.FaoInstitute;
 import org.genesys2.server.model.impl.ITPGRFAStatus;
@@ -38,6 +35,9 @@ import org.genesys2.server.persistence.domain.CountryRepository;
 import org.genesys2.server.persistence.domain.ITPGRFAStatusRepository;
 import org.genesys2.server.service.ContentService;
 import org.genesys2.server.service.GeoService;
+import org.genesys2.server.service.worker.CountryInfo;
+import org.genesys2.server.service.worker.DavrosCountrySource;
+import org.genesys2.server.service.worker.GeoNamesCountrySource;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.cache.annotation.CacheEvict;
 import org.springframework.data.domain.PageRequest;
@@ -65,6 +65,12 @@ public class GeoServiceImpl implements GeoService {
 	@Autowired
 	ITPGRFAStatusRepository itpgrfaRepository;
 
+	@Autowired
+	private GeoNamesCountrySource geoNamesCountrySource;
+
+	@Autowired
+	private DavrosCountrySource davrosCountrySource;
+
 	@Override
 	public List<Country> listAll() {
 		return countryRepository.findAll(new Sort("name", "current"));
@@ -210,7 +216,7 @@ public class GeoServiceImpl implements GeoService {
 	}
 
 	private void updateDavrosCountries() throws IOException {
-		final List<CountryInfo> countries = DavrosCountrySource.fetchCountryData();
+		final List<CountryInfo> countries = davrosCountrySource.fetchCountryData();
 
 		if (LOG.isDebugEnabled()) {
 			LOG.debug("Got " + countries.size() + " countries from remote source.");
@@ -272,7 +278,7 @@ public class GeoServiceImpl implements GeoService {
 	}
 
 	private void updateGeoNamesCountries() throws IOException {
-		final List<CountryInfo> countries = GeoNamesCountrySource.fetchCountryData();
+		final List<CountryInfo> countries = geoNamesCountrySource.fetchCountryData();
 
 		if (LOG.isDebugEnabled()) {
 			LOG.debug("Got " + countries.size() + " countries from remote source.");

src/main/java/org/genesys2/server/service/worker/CountryAlternateNamesUpdater.java

@@ -29,15 +29,14 @@ import java.util.zip.ZipInputStream;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.time.StopWatch;
-import org.apache.commons.lang3.ArrayUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.ClientProtocolException;
-import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpGet;
-import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
 import org.genesys2.server.model.impl.Country;
 import org.genesys2.server.service.CountryNamesUpdater;
 import org.genesys2.server.service.GeoService;
@@ -69,6 +68,9 @@ public class CountryAlternateNamesUpdater implements CountryNamesUpdater {
 
 	public static final Log LOG = LogFactory.getLog(CountryAlternateNamesUpdater.class);
 
+	@Autowired
+	private HttpClientBuilder httpClientBuilder;
+
 	@Autowired
 	private InstituteService instituteService;
 
@@ -89,7 +91,7 @@ public class CountryAlternateNamesUpdater implements CountryNamesUpdater {
 	public void updateAlternateNames() throws IOException {
 		InputStream alternateNamesStream = null;
 
-		final HttpClient httpclient = new DefaultHttpClient();
+		final CloseableHttpClient httpclient = httpClientBuilder.build();
 		final HttpGet httpget = new HttpGet(ALTERNATE_NAMES_URL);
 		HttpResponse response = null;
 		try {
@@ -121,23 +123,20 @@ public class CountryAlternateNamesUpdater implements CountryNamesUpdater {
 
 				if (alternateNamesStream == null) {
 					LOG.warn("Didn't find alternateNames.zip, stopping update.");
-					IOUtils.closeQuietly(instream);
-					return;
+				} else {
+					updateFromStream(alternateNamesStream);
 				}
 			}
+
 		} catch (final ClientProtocolException e) {
 			LOG.error(e.getMessage(), e);
 			throw new IOException(e);
-		} finally {
-		}
-
-		try {
-			updateFromStream(alternateNamesStream);
 		} catch (final IOException e) {
 			LOG.error(e);
 			throw e;
 		} finally {
 			IOUtils.closeQuietly(alternateNamesStream);
+			IOUtils.closeQuietly(httpclient);
 		}
 	}
 
@@ -238,9 +237,9 @@ public class CountryAlternateNamesUpdater implements CountryNamesUpdater {
 				final ObjectNode jsonNames = objectMapper.createObjectNode();
 
 				for (final String[] line : batchCopy) {
-					if (refnameId == 614540) {
-						System.err.println(ArrayUtils.toString(line));
-					}
+					// if (refnameId == 614540) {
+					// System.err.println(ArrayUtils.toString(line));
+					// }
 					final boolean preferred = line.length > 4 && "1".equals(line[4]);
 					final boolean colloquial = line.length > 6 && "1".equals(line[6]);
 					final boolean historic = line.length > 7 && "1".equals(line[7]);

src/main/java/org/genesys2/geo/sources/CountryInfo.java → src/main/java/org/genesys2/server/service/worker/CountryInfo.java

@@ -14,7 +14,7 @@
  * limitations under the License.
  **/
 
-package org.genesys2.geo.sources;
+package org.genesys2.server.service.worker;
 
 import java.io.Serializable;
 import java.text.MessageFormat;

src/main/java/org/genesys2/geo/sources/DavrosCountrySource.java → src/main/java/org/genesys2/server/service/worker/DavrosCountrySource.java

@@ -14,7 +14,7 @@
  * limitations under the License.
  **/
 
-package org.genesys2.geo.sources;
+package org.genesys2.server.service.worker;
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -29,9 +29,11 @@ import org.apache.commons.logging.LogFactory;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.ClientProtocolException;
-import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpGet;
-import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
 
 /**
  * Fetch and parse country information from
@@ -39,38 +41,37 @@ import org.apache.http.impl.client.DefaultHttpClient;
  *
  * @author mobreza
  */
+@Component
 public class DavrosCountrySource {
 	private static final String DAVROS_ISO3166_URL = "http://www.davros.org/misc/iso3166.txt";
 
 	public static final Log LOG = LogFactory.getLog(DavrosCountrySource.class);
 
+	@Autowired
+	private HttpClientBuilder httpClientBuilder;
+
 	/**
 	 * Retrieve data from davros.org
 	 *
 	 * @throws IOException
 	 */
-	public static List<CountryInfo> fetchCountryData() throws IOException {
+	public List<CountryInfo> fetchCountryData() throws IOException {
 
-		final HttpClient httpclient = new DefaultHttpClient();
+		final CloseableHttpClient httpclient = httpClientBuilder.build();
 		final HttpGet httpget = new HttpGet(DAVROS_ISO3166_URL);
 
 		HttpResponse response = null;
+		InputStream instream = null;
+		
 		try {
 			response = httpclient.execute(httpget);
-		} catch (final ClientProtocolException e) {
-			LOG.error(e.getMessage(), e);
-			throw new IOException("Could not execute HTTP request: " + e.getMessage(), e);
-		}
 
-		LOG.debug(response.getStatusLine());
+			LOG.debug(response.getStatusLine());
 
-		// Get hold of the response entity
-		final HttpEntity entity = response.getEntity();
-		LOG.debug(entity.getContentType() + " " + entity.getContentLength());
+			// Get hold of the response entity
+			final HttpEntity entity = response.getEntity();
+			LOG.debug(entity.getContentType() + " " + entity.getContentLength());
 
-		InputStream instream = null;
-
-		try {
 			instream = entity.getContent();
 			final BufferedReader inreader = new BufferedReader(new InputStreamReader(instream));
 
@@ -104,6 +105,9 @@ public class DavrosCountrySource {
 			LOG.info("Got " + countries.size() + " countries!");
 			return countries;
 
+		} catch (final ClientProtocolException e) {
+			LOG.error(e.getMessage(), e);
+			throw new IOException("Could not execute HTTP request: " + e.getMessage(), e);
 		} catch (final RuntimeException ex) {
 			LOG.error(ex.getMessage(), ex);
 			httpget.abort();
@@ -111,6 +115,7 @@ public class DavrosCountrySource {
 		} finally {
 			LOG.info("Done fetching country info from davros.org");
 			IOUtils.closeQuietly(instream);
+			IOUtils.closeQuietly(httpclient);
 		}
 	}
 

src/main/java/org/genesys2/geo/sources/GeoNamesCountrySource.java → src/main/java/org/genesys2/server/service/worker/GeoNamesCountrySource.java

@@ -14,7 +14,7 @@
  * limitations under the License.
  **/
 
-package org.genesys2.geo.sources;
+package org.genesys2.server.service.worker;
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -31,9 +31,11 @@ import org.apache.commons.logging.LogFactory;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.ClientProtocolException;
-import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpGet;
-import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
 
 import au.com.bytecode.opencsv.CSVReader;
 
@@ -43,38 +45,37 @@ import au.com.bytecode.opencsv.CSVReader;
  *
  * @author mobreza
  */
+@Component
 public class GeoNamesCountrySource {
 	private static final String GEONAMES_ISO3166_URL = "http://download.geonames.org/export/dump/countryInfo.txt";
 
 	public static final Log LOG = LogFactory.getLog(GeoNamesCountrySource.class);
 
+	@Autowired
+	private HttpClientBuilder httpClientBuilder;
+
 	/**
 	 * Retrieve data from davros.org
 	 *
 	 * @throws IOException
 	 */
-	public static List<CountryInfo> fetchCountryData() throws IOException {
+	public List<CountryInfo> fetchCountryData() throws IOException {
 
-		final HttpClient httpclient = new DefaultHttpClient();
 		final HttpGet httpget = new HttpGet(GEONAMES_ISO3166_URL);
 
 		HttpResponse response = null;
+		InputStream instream = null;
+
+		final CloseableHttpClient httpclient = httpClientBuilder.build();
 		try {
 			response = httpclient.execute(httpget);
-		} catch (final ClientProtocolException e) {
-			LOG.error(e.getMessage(), e);
-			throw new IOException("Could not execute HTTP request: " + e.getMessage(), e);
-		}
 
-		LOG.debug(response.getStatusLine());
+			LOG.debug(response.getStatusLine());
 
-		// Get hold of the response entity
-		final HttpEntity entity = response.getEntity();
-		LOG.debug(entity.getContentType() + " " + entity.getContentLength());
+			// Get hold of the response entity
+			final HttpEntity entity = response.getEntity();
+			LOG.debug(entity.getContentType() + " " + entity.getContentLength());
 
-		InputStream instream = null;
-
-		try {
 			instream = entity.getContent();
 			final BufferedReader inreader = new BufferedReader(new InputStreamReader(instream));
 
@@ -114,14 +115,17 @@ public class GeoNamesCountrySource {
 
 			LOG.info("Got " + countries.size() + " countries!");
 			return countries;
-
+		} catch (final ClientProtocolException e) {
+			LOG.error(e.getMessage(), e);
+			throw new IOException("Could not execute HTTP request: " + e.getMessage(), e);
 		} catch (final RuntimeException ex) {
 			LOG.error(ex.getMessage(), ex);
 			httpget.abort();
 			throw new IOException(ex);
 		} finally {
-			LOG.info("Done fetching country info from davros.org");
+			LOG.info("Done fetching country info from geonames.org");
 			IOUtils.closeQuietly(instream);
+			IOUtils.closeQuietly(httpclient);
 		}
 	}
 }

src/main/java/org/genesys2/server/service/worker/ITPGRFAStatusUpdater.java

@@ -33,9 +33,9 @@ import org.apache.commons.logging.LogFactory;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.ClientProtocolException;
-import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpGet;
-import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
 import org.genesys2.server.model.impl.Country;
 import org.genesys2.server.model.impl.ITPGRFAStatus;
 import org.genesys2.server.service.GeoService;
@@ -64,6 +64,9 @@ public class ITPGRFAStatusUpdater {
 	@Autowired
 	private TaskExecutor taskExecutor;
 
+	@Autowired
+	private HttpClientBuilder httpClientBuilder;
+
 	private static final int BATCH_SIZE = 20;
 
 	private static final String[] CSV_HEADERS = { "Country ISO CODE", "Country", "FAO Region 1", "FAO Region 2", "CP", "Name of NFP", "Membership", "by",
@@ -83,9 +86,9 @@ public class ITPGRFAStatusUpdater {
 	public void downloadAndUpdate() throws IOException {
 		InputStream itpgrfaCSVStream = null;
 
-		final HttpClient httpclient = new DefaultHttpClient();
 		final HttpGet httpget = new HttpGet(ITPGRFA_STATUS_URL);
 		HttpResponse response = null;
+		final CloseableHttpClient httpclient = httpClientBuilder.build();
 		try {
 			response = httpclient.execute(httpget);
 
@@ -98,20 +101,18 @@ public class ITPGRFAStatusUpdater {
 			if (entity != null) {
 				itpgrfaCSVStream = new BufferedInputStream(entity.getContent());
 			}
+
+			updateFromStream(itpgrfaCSVStream);
+		
 		} catch (final ClientProtocolException e) {
 			LOG.error(e.getMessage(), e);
 			throw new IOException(e);
-		} finally {
-
-		}
-
-		try {
-			updateFromStream(itpgrfaCSVStream);
 		} catch (final IOException e) {
 			LOG.error(e);
 			throw e;
 		} finally {
 			IOUtils.closeQuietly(itpgrfaCSVStream);
+			IOUtils.closeQuietly(httpclient);
 		}
 	}
 

src/main/java/org/genesys2/server/service/worker/InstituteUpdater.java

@@ -34,10 +34,9 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
-import org.apache.http.client.ClientProtocolException;
-import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpGet;
-import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
 import org.genesys2.server.model.impl.FaoInstitute;
 import org.genesys2.server.service.GeoService;
 import org.genesys2.server.service.InstituteService;
@@ -62,6 +61,9 @@ public class InstituteUpdater {
 	@Autowired
 	private TaskExecutor taskExecutor;
 
+	@Autowired
+	private HttpClientBuilder httpClientBuilder;
+
 	private static final int BATCH_SIZE = 50;
 
 	/**
@@ -71,79 +73,79 @@ public class InstituteUpdater {
 	 */
 	public void updateFaoInstitutes() throws IOException {
 
-		final HttpClient httpclient = new DefaultHttpClient();
 		final HttpGet httpget = new HttpGet(WIEWS_EXPORT_URL);
 
 		HttpResponse response = null;
+
+		final CloseableHttpClient httpclient = httpClientBuilder.build();
 		try {
 			response = httpclient.execute(httpget);
-		} catch (final ClientProtocolException e) {
-			LOG.error(e.getMessage(), e);
-			throw new IOException(e);
-		}
 
-		LOG.debug(response.getStatusLine());
-
-		// Get hold of the response entity
-		final HttpEntity entity = response.getEntity();
-		LOG.debug(entity.getContentType() + " " + entity.getContentLength());
-
-		// If the response does not enclose an entity, there is no
-		// need
-		// to bother about connection release
-		if (entity != null) {
-			ZipInputStream instream = null;
-
-			try {
-				instream = new ZipInputStream(entity.getContent());
-				final ZipEntry zipEntry = instream.getNextEntry();
-				LOG.debug("Got entry: " + zipEntry.getName());
-				if (!zipEntry.getName().equals("export_c.txt")) {
-					LOG.warn("Expected export_c, not " + zipEntry.getName());
-					throw new IOException("Missing export_c");
-				}
+			LOG.debug(response.getStatusLine());
+
+			// Get hold of the response entity
+			final HttpEntity entity = response.getEntity();
+			LOG.debug(entity.getContentType() + " " + entity.getContentLength());
+
+			// If the response does not enclose an entity, there is no
+			// need
+			// to bother about connection release
+			if (entity != null) {
+				ZipInputStream instream = null;
+
+				try {
+					instream = new ZipInputStream(entity.getContent());
+					final ZipEntry zipEntry = instream.getNextEntry();
+					LOG.debug("Got entry: " + zipEntry.getName());
+					if (!zipEntry.getName().equals("export_c.txt")) {
+						LOG.warn("Expected export_c, not " + zipEntry.getName());
+						throw new IOException("Missing export_c");
+					}
 
-				final InputStreamReader inreader = new InputStreamReader(instream, "UTF-8");
-				final CSVReader reader = new CSVReader(inreader, ',', '"', false);
+					final InputStreamReader inreader = new InputStreamReader(instream, "UTF-8");
+					final CSVReader reader = new CSVReader(inreader, ',', '"', false);
 
-				final Map<String, String[]> batch = new HashMap<String, String[]>(BATCH_SIZE);
+					final Map<String, String[]> batch = new HashMap<String, String[]>(BATCH_SIZE);
 
-				String[] line = null;
-				while ((line = reader.readNext()) != null) {
-					for (int i = 0; i < line.length; i++) {
-						if (line[i].equals("null") || StringUtils.isBlank(line[i])) {
-							line[i] = null;
+					String[] line = null;
+					while ((line = reader.readNext()) != null) {
+						for (int i = 0; i < line.length; i++) {
+							if (line[i].equals("null") || StringUtils.isBlank(line[i])) {
+								line[i] = null;
+							}
 						}
-					}
-					// if (StringUtils.isNotBlank(line[14])) {
-					// LOG.info(ArrayUtils.toString(line));
-					// }
+						// if (StringUtils.isNotBlank(line[14])) {
+						// LOG.info(ArrayUtils.toString(line));
+						// }
 
-					final String instCode = line[0];
-					batch.put(instCode, line);
+						final String instCode = line[0];
+						batch.put(instCode, line);
 
-					if (batch.size() == BATCH_SIZE) {
-						workIt(batch);
-						batch.clear();
-					}
+						if (batch.size() == BATCH_SIZE) {
+							workIt(batch);
+							batch.clear();
+						}
 
+					}
+					workIt(batch);
+					batch.clear();
+