Merge pull request #401 from bullhorn/log4j-upgrade

Log4j upgrade

pom.xml

@@ -5,7 +5,7 @@
 
     <groupId>com.bullhorn</groupId>
     <artifactId>sdk-rest</artifactId>
-    <version>1.4.58</version>
+    <version>1.4.59</version>
     <packaging>jar</packaging>
 
     <name>Bullhorn REST SDK</name>
@@ -161,9 +161,9 @@
         </dependency>
 
         <dependency>
-            <groupId>log4j</groupId>
-            <artifactId>log4j</artifactId>
-            <version>1.2.17</version>
+            <groupId>org.apache.logging.log4j</groupId>
+            <artifactId>log4j-core</artifactId>
+            <version>2.19.0</version>
         </dependency>
 
         <dependency>
@@ -193,12 +193,6 @@
             <version>2.24.0</version>
         </dependency>
 
-        <dependency>
-            <groupId>de.javakaffee</groupId>
-            <artifactId>kryo-serializers</artifactId>
-            <version>0.37</version>
-        </dependency>
-
         <dependency>
             <groupId>org.hamcrest</groupId>
             <artifactId>hamcrest</artifactId>

src/main/java/com/bullhornsdk/data/api/StandardBullhornData.java

@@ -15,7 +15,8 @@
 import java.util.stream.Collectors;
 
 import org.apache.commons.lang3.StringUtils;
-import org.apache.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.springframework.http.HttpEntity;
@@ -142,7 +143,7 @@
  */
 
 public class StandardBullhornData implements BullhornData {
-    protected static Logger log = Logger.getLogger(StandardBullhornData.class);
+    protected static Logger log = LogManager.getLogger(StandardBullhornData.class);
 
     protected final RestApiSession restSession;
 
@@ -1451,11 +1452,9 @@ protected ParsedResume handleResumeParseError(int tryNumber, HttpStatusCodeExcep
             response = new StandardParsedResume();
             response.setErrorCode(error.getStatusCode().name());
             response.setErrorMessage("BH api responded with the following message: " + error.getResponseBodyAsString());
-            log.error("Failed to parse resume after " + RESUME_PARSE_RETRY + " tries. Response body from bh rest apis = "
-                    + error.getResponseBodyAsString());
+            log.error("Failed to parse resume after {} tries. Response body from bh rest apis = {}", RESUME_PARSE_RETRY, error.getResponseBodyAsString());
         } else {
-            log.info(error.getResponseBodyAsString() + " Try " + tryNumber + " out of " + RESUME_PARSE_RETRY
-                    + ". Trying again. Response body from bh rest apis = " + error.getResponseBodyAsString());
+            log.info("{} Try {} out of {}. Trying again. Response body from bh rest apis = {}", error.getResponseBodyAsString(), tryNumber, RESUME_PARSE_RETRY, error.getResponseBodyAsString());
         }
 
         return response;
@@ -1634,7 +1633,7 @@ protected FileWrapper handleGetFileContentWithMetaData(Class<? extends FileEntit
 
             fileWrapper = new StandardFileWrapper(fileContent, correctMetaData);
         } catch (Exception e) {
-            log.error("Error getting file with id: " + fileId + " for " + type.getSimpleName() + " with id:" + entityId);
+            log.error("Error getting file with id: {} for {} with id:{}", fileId, type.getSimpleName(), entityId);
         }
 
         return fileWrapper;
@@ -2144,9 +2143,9 @@ protected boolean handleHttpStatusCodeError(Map<String, String> uriVariables, in
         } else if (error.getStatusCode() == HttpStatus.TOO_MANY_REQUESTS) {
             isTooManyRequestsError = true;
         }
-        log.error(
-                "HttpStatusCodeError making api call. Try number:" + tryNumber + " out of " + API_RETRY + ". Http status code: "
-                        + error.getStatusCode() + ". Response body: " + error.getResponseBodyAsString(), error);
+        String errorMessage = "HttpStatusCodeError making api call. Try number:" + tryNumber + " out of " + API_RETRY + ". Http status code: "
+            + error.getStatusCode() + ". Response body: " + error.getResponseBodyAsString();
+        log.error(errorMessage, error);
         if (tryNumber >= API_RETRY && !isTooManyRequestsError) {
             throw new RestApiException("HttpStatusCodeError making api call with url variables " + uriVariables.toString()
                     + ". Http status code: " + error.getStatusCode().toString() + ". Response body: " + error == null ? ""
@@ -2156,7 +2155,8 @@ protected boolean handleHttpStatusCodeError(Map<String, String> uriVariables, in
     }
 
     protected void handleApiError(int tryNumber, Exception e) {
-        log.error("Error making api call. Try number:" + tryNumber + " out of " + API_RETRY, e);
+        String message = "Error making api call. Try number:" + tryNumber + " out of " + API_RETRY;
+        log.error(message, e);
     }
 
     protected void resetBhRestToken(Map<String, String> uriVariables) {

src/main/java/com/bullhornsdk/data/api/helper/EntityUpdateWorker.java

@@ -2,7 +2,8 @@
 
 import java.util.concurrent.Callable;
 
-import org.apache.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import com.bullhornsdk.data.api.BullhornData;
 import com.bullhornsdk.data.exception.RestApiException;
@@ -14,7 +15,7 @@ public class EntityUpdateWorker<C extends CrudResponse> implements Callable<C> {
 
 	private final BullhornData bullhornApiRest;
 	private final UpdateEntity entity;
-	private final Logger log = Logger.getLogger(EntityUpdateWorker.class);
+	private final Logger log = LogManager.getLogger(EntityUpdateWorker.class);
 
 	public EntityUpdateWorker(BullhornData bullhornApiRest, UpdateEntity entity) {
 		super();
@@ -28,9 +29,8 @@ public C call() throws Exception {
 		try {
 			response = bullhornApiRest.updateEntity(entity);
 		} catch (RestApiException e) {
-			log.error(
-					"Error updating entity of type " + entity.getClass().getSimpleName() + " with id " + entity.getId(),
-					e);
+            String message = "Error updating entity of type " + entity.getClass().getSimpleName() + " with id " + entity.getId();
+			log.error(message, e);
 		}
 		return response;
 	}

src/main/java/com/bullhornsdk/data/api/helper/RestApiSession.java

@@ -13,7 +13,8 @@
 import org.apache.commons.httpclient.methods.GetMethod;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
 import org.json.JSONException;
@@ -52,7 +53,7 @@ public class RestApiSession {
 
 	private static final String REFRESH_TOKEN_GRANT_TYPE = "refresh_token";
 
-	private static Logger log = Logger.getLogger(RestApiSession.class);
+	private static Logger log = LogManager.getLogger(RestApiSession.class);
 
 	private RestTemplate restTemplate;
 
@@ -137,7 +138,8 @@ private void createSession() {
 				break;
 			} catch (Exception e) {
 				if (tryNumber < SESSION_RETRY) {
-					log.error("Error creating REST session. Try number: " + tryNumber + " out of " + SESSION_RETRY + " trying again.", e);
+                    String message = "Error creating REST session. Try number: " + tryNumber + " out of " + SESSION_RETRY + " trying again.";
+					log.error(message, e);
 				} else {
 					log.error("Final error creating REST session. Shutting down.", e);
 
@@ -244,9 +246,9 @@ private void login() {
 
 			restUrl = (String) responseJson.get("restUrl");
 		} catch (RestClientException | IOException e) {
-			log.error("Failed to login. " + responseJson, e);
-
-			throw new RestApiException("Failed to login and get BhRestToken: " + responseJson, e);
+            String message = "Failed to login and get BhRestToken: " + responseJson;
+			log.error(message, e);
+			throw new RestApiException(message, e);
 		}
 	}
 
@@ -426,9 +428,10 @@ private synchronized JSONObject getLoginInfoFromApi() {
 
             return this.loginInfo;
         } catch(RestClientException | JSONException e) {
-            log.error("Error occurred dynamically determining REST urls with username " + restCredentials.getUsername(), e);
+            String message = "Error occurred dynamically determining REST urls with username " + restCredentials.getUsername();
+            log.error(message, e);
 
-            throw new RestApiException("Failed to dynamically determine REST urls with username " + restCredentials.getUsername());
+            throw new RestApiException(message);
         }
     }
 

src/main/java/com/bullhornsdk/data/api/helper/RestFileManager.java

@@ -5,7 +5,8 @@
 
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.springframework.core.io.FileSystemResource;
 import org.springframework.util.FileCopyUtils;
 import org.springframework.util.LinkedMultiValueMap;
@@ -20,7 +21,7 @@ public class RestFileManager {
 
     private final String formFileName = "file";
 
-    private static Logger log = Logger.getLogger(RestFileManager.class);
+    private static Logger log = LogManager.getLogger(RestFileManager.class);
 
     /**
      * Will create a unique sub folder in the temp directory. This is done so that we can name the file with the original file
@@ -80,7 +81,7 @@ public void deleteTempFile(MultiValueMap<String, Object> multiValueMap) {
         try {
             FileUtils.deleteDirectory(newFolder);
         } catch (IOException e) {
-            log.info("Unable to delete temp file " + filePath);
+            log.info("Unable to delete temp file {}", filePath);
         }
 
     }
@@ -103,7 +104,7 @@ public void deleteTempResume(MultiValueMap<String, Object> multiValueMap) {
         File file = fileSystemResource.getFile();
 
         if (!file.delete()) {
-            log.info("Unable to delete temp resume " + file.getAbsolutePath());
+            log.info("Unable to delete temp resume {}", file.getAbsolutePath());
         }
     }
 

src/main/java/com/bullhornsdk/data/api/helper/RestJsonConverter.java

@@ -2,7 +2,8 @@
 
 import java.io.IOException;
 
-import org.apache.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import com.bullhornsdk.data.exception.RestMappingException;
 import com.bullhornsdk.data.model.entity.core.type.BullhornEntity;
@@ -16,7 +17,7 @@
 
 public class RestJsonConverter {
 
-    private static Logger log = Logger.getLogger(RestJsonConverter.class);
+    private static Logger log = LogManager.getLogger(RestJsonConverter.class);
 
     private final ObjectMapper objectMapperWrapped;
 
@@ -109,7 +110,8 @@ public <T extends BullhornEntity> String convertEntityToJsonString(T entity) {
         try {
             jsonString = objectMapperStandard.writeValueAsString(entity);
         } catch (JsonProcessingException e) {
-            log.error("Error deserializing entity of type" + entity.getClass() + " to jsonString.", e);
+            String message = "Error deserializing entity of type" + entity.getClass() + " to jsonString.";
+            log.error(message, e);
         }
         return jsonString;
     }

src/main/java/com/bullhornsdk/data/api/helper/concurrency/standard/RestConcurrencyService.java

@@ -9,13 +9,14 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 
-import org.apache.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import com.bullhornsdk.data.api.helper.concurrency.ConcurrencyService;
 
 public class RestConcurrencyService implements ConcurrencyService {
 
-	private static Logger log = Logger.getLogger(RestConcurrencyService.class);
+	private static Logger log = LogManager.getLogger(RestConcurrencyService.class);
 
 	@Override
 	public <T, C extends Callable<T>> List<T> spinThreadsAndWaitForResult(List<C> taskList) {

src/main/java/com/bullhornsdk/data/api/mock/MockBullhornData.groovy

@@ -31,7 +31,8 @@ import com.bullhornsdk.data.model.response.resume.ParsedResume
 import com.bullhornsdk.data.model.response.resume.ParsedResumeAsEntity
 import com.bullhornsdk.data.model.response.subscribe.SubscribeToEventsResponse
 import com.bullhornsdk.data.model.response.subscribe.standard.StandardSubscribeToEventsResponse
-import org.apache.log4j.Logger
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.joda.time.DateTime
 import org.springframework.web.multipart.MultipartFile
 
@@ -48,7 +49,7 @@ public class MockBullhornData implements BullhornData {
     private final MockDataHandler mockDataHandler;
     private final RestErrorHandler restErrorHandler;
     private final RestApiSession restSession;
-    private final Logger log = Logger.getLogger(MockBullhornData.class);
+    private final Logger log = LogManager.getLogger(MockBullhornData.class);
     protected Boolean executeFormTriggers = false;
 
 

src/main/java/com/bullhornsdk/data/api/mock/MockDataHandler.groovy

@@ -40,7 +40,8 @@ import com.bullhornsdk.data.model.response.resume.standard.StandardParsedResume
 import com.bullhornsdk.data.model.response.resume.standard.StandardParsedResumeAsEntity
 import com.bullhornsdk.data.util.copy.KryoObjectCopyHelper
 import org.apache.commons.lang3.StringUtils
-import org.apache.log4j.Logger
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.codehaus.groovy.runtime.NullObject
 import org.joda.time.DateTime
 import org.springframework.web.multipart.MultipartFile
@@ -61,7 +62,7 @@ import java.io.File as JavaFile
 
 public class MockDataHandler {
 
-	private final static Logger log = Logger.getLogger(MockDataHandler.class);
+	private final static Logger log = LogManager.getLogger(MockDataHandler.class);
 	private final MockDataLoader mockDataLoader;
 	private Map<Class<? extends BullhornEntity>, Map<Integer, ? extends BullhornEntity>> restEntityMap;
 	private Map<Class<? extends BullhornEntity>, MetaData<?>> restMetaDataMap;
@@ -232,9 +233,10 @@ public class MockDataHandler {
 		try {
 			updateExistingEntityWithNewNonNullValues(entity, existingEntity);
 		} catch (Exception e) {
+            String message = "Error updating entity of type: " + entity.getClass().getSimpleName() + " with id: " + entity.getId();
 			response.setErrorCode("500");
-			response.setErrorMessage("Error updating entity of type: " + entity.getClass().getSimpleName() + " with id: " + entity.getId());
-			log.error("Error updating entity of type: " + entity.getClass().getSimpleName() + " with id: " + entity.getId(), e);
+			response.setErrorMessage(message);
+			log.error(message, e);
 		}
 
 		return (C) response;
@@ -896,7 +898,6 @@ public class MockDataHandler {
 			}else {
 			   containsValue = (field1 == value1);
 			}
-			//log.info("field1 = "+field1+" value1 = "+value1+" containsValue = "+containsValue);
 			return containsValue;
 		};
 
@@ -1100,7 +1101,7 @@ public class MockDataHandler {
             try {
                 setValueFromPath(copyOfFromEntity,toEntity,fullPath)
             } catch(MissingPropertyException e) {
-                log.error("Missing property " + e.getProperty() + " on entity " + fromEntity.getClass().getSimpleName());
+                log.error("Missing property {} on entity {}", e.getProperty(), fromEntity.getClass().getSimpleName());
             }
 		}
 
@@ -1134,7 +1135,7 @@ public class MockDataHandler {
 				try {
                     setValueFromPath(fromProperty,toProperty, partialPath)
                 } catch(MissingPropertyException e) {
-                    log.error("Missing property " + e.getProperty() + " on entity " + to.getClass().getSimpleName());
+                    log.error("Missing property {} on entity {}", e.getProperty(), to.getClass().getSimpleName());
                 }
 
                 if(!parentPropertyIsOneToMany(fromProperty,toProperty,path)){
@@ -1144,11 +1145,11 @@ public class MockDataHandler {
 			}
 
 		}else if(fromProperty == null || fromProperty instanceof NullObject){
-			log.debug("fromProperty is null, no need to set the toProperty then. ");
+			log.debug("fromProperty is null, no need to set the toProperty then.");
 		}else if(parentPropertyIsOneToMany(fromProperty,toProperty,path)){
 			//All fields on the OneToMany are set under propertyIsOneToMany.
 		    //Field selection for OneToMany is not currently supported.
-		    log.debug("parent is OneToMany: "+path);
+		    log.debug("parent is OneToMany: {}", path);
 		}else if(propertyIsOneToMany(fromProperty,toProperty,path)){
 			toProperty?."${path}" = fromProperty?."${path}";
 		}else if(propertyIsNullNestedRestEntity(fromProperty,toProperty,path)){

src/main/java/com/bullhornsdk/data/api/mock/MockDataLoader.groovy

@@ -71,14 +71,15 @@ import com.bullhornsdk.data.model.response.list.ListWrapper
 import com.bullhornsdk.data.model.response.list.PropertyOptionsListWrapper
 import com.bullhornsdk.data.util.copy.KryoObjectCopyHelper
 import org.apache.commons.io.IOUtils
-import org.apache.log4j.Logger
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.springframework.core.io.ClassPathResource
 
 import java.util.concurrent.ConcurrentHashMap
 
 public class MockDataLoader {
 
-    private final static Logger log = Logger.getLogger(MockDataLoader.class);
+    private final static Logger log = LogManager.getLogger(MockDataLoader.class);
     private final RestJsonConverter restJsonConverter;
 
     private Map<Class<? extends BullhornEntity>, Map<Integer, ? extends BullhornEntity>> restEntityMapCache;
@@ -346,7 +347,7 @@ public class MockDataLoader {
             jsonDataString = IOUtils.toString(data.getInputStream(), "UTF-8");
 
         } catch (IOException e) {
-            log.error("Unable to load test data from filename: " + fileName);
+            log.error("Unable to load test data from filename: {}", fileName);
             throw new IllegalArgumentException("Unable to load test data from filename: " + fileName, e);
         }