fix #2743 code cleanup

src/main/java/org/codelibs/fess/api/json/SearchApiManager.java

@@ -124,7 +124,8 @@ public class SearchApiManager extends BaseApiManager {
             if (values.length > 5 && "favorite".equals(values[5])) {
                 request.setAttribute(DOC_ID_FIELD, values[4]);
                 return FormatType.FAVORITE;
-            } else if (values.length > 4 && "all".equals(values[4])) {
+            }
+            if (values.length > 4 && "all".equals(values[4])) {
                 return FormatType.SCROLL;
             }
             return FormatType.SEARCH;

src/main/java/org/codelibs/fess/app/service/FailureUrlService.java

@@ -15,13 +15,15 @@
  */
 package org.codelibs.fess.app.service;
 
+import java.io.IOException;
 import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.util.List;
 
 import javax.annotation.Resource;
 
-import org.apache.commons.io.output.StringBuilderWriter;
-import org.apache.commons.lang3.StringUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.codelibs.core.beans.util.BeanUtil;
 import org.codelibs.core.lang.StringUtil;
 import org.codelibs.fess.Constants;
@@ -39,6 +41,8 @@ import org.dbflute.optional.OptionalEntity;
 
 public class FailureUrlService {
 
+    private static final Logger logger = LogManager.getLogger(FailureUrlService.class);
+
     @Resource
     protected FailureUrlBhv failureUrlBhv;
 
@@ -149,7 +153,7 @@ public class FailureUrlService {
         });
 
         failureUrl.setErrorName(errorName);
-        failureUrl.setErrorLog(StringUtils.abbreviate(getStackTrace(e), 4000));
+        failureUrl.setErrorLog(getStackTrace(e));
         failureUrl.setLastAccessTime(ComponentUtil.getSystemHelper().getCurrentTimeAsLong());
         failureUrl.setThreadName(Thread.currentThread().getName());
 
@@ -160,9 +164,13 @@ public class FailureUrlService {
 
     private String getStackTrace(final Throwable t) {
         final SystemHelper systemHelper = ComponentUtil.getSystemHelper();
-        final StringBuilderWriter sw = new StringBuilderWriter();
-        final PrintWriter pw = new PrintWriter(sw, true);
-        t.printStackTrace(pw);
-        return systemHelper.abbreviateLongText(sw.toString());
+        try (final StringWriter sw = new StringWriter(); final PrintWriter pw = new PrintWriter(sw)) {
+            t.printStackTrace(pw);
+            pw.flush();
+            return systemHelper.abbreviateLongText(sw.toString());
+        } catch (final IOException e) {
+            logger.warn("Failed to print the stack trace {}", t.getMessage(), e);
+        }
+        return StringUtil.EMPTY;
     }
 }

src/main/java/org/codelibs/fess/app/service/SearchLogService.java

@@ -556,14 +556,14 @@ public class SearchLogService {
     }
 
     public void deleteSearchLog(final Object e) {
-        if (e instanceof ClickLog) {
-            clickLogBhv.delete((ClickLog) e);
-        } else if (e instanceof FavoriteLog) {
-            favoriteLogBhv.delete((FavoriteLog) e);
-        } else if (e instanceof UserInfo) {
-            userInfoBhv.delete((UserInfo) e);
-        } else if (e instanceof SearchLog) {
-            searchLogBhv.delete((SearchLog) e);
+        if (e instanceof final ClickLog clickLog) {
+            clickLogBhv.delete(clickLog);
+        } else if (e instanceof final FavoriteLog favoriteLog) {
+            favoriteLogBhv.delete(favoriteLog);
+        } else if (e instanceof final UserInfo userInfo) {
+            userInfoBhv.delete(userInfo);
+        } else if (e instanceof final SearchLog searchLog) {
+            searchLogBhv.delete(searchLog);
         } else {
             throw new FessSystemException("Unknown log entity: " + e);
         }

src/main/java/org/codelibs/fess/crawler/transformer/AbstractFessFileTransformer.java

@@ -148,7 +148,7 @@ public abstract class AbstractFessFileTransformer extends AbstractTransformer im
                                 if (dt != null) {
                                     dataMap.put(mapping.getValue1(), FessFunctions.formatDate(dt));
                                 } else {
-                                    logger.warn("Failed to parse {}", mapping.toString());
+                                    logger.warn("Failed to parse {}", mapping);
                                 }
                             } else {
                                 logger.warn("Unknown mapping type: {}={}", key, mapping);

src/main/java/org/codelibs/fess/dict/DictionaryCreator.java

@@ -30,7 +30,7 @@ public abstract class DictionaryCreator {
     @Resource
     protected DictionaryManager dictionaryManager;
 
-    public DictionaryCreator(final String pattern) {
+    protected DictionaryCreator(final String pattern) {
         this.pattern = Pattern.compile(pattern);
     }
 

src/main/java/org/codelibs/fess/dict/DictionaryFile.java

@@ -38,7 +38,7 @@ public abstract class DictionaryFile<T extends DictionaryItem> {
 
     protected Date timestamp;
 
-    public DictionaryFile(final String id, final String path, final Date timestamp) {
+    protected DictionaryFile(final String id, final String path, final Date timestamp) {
         this.id = id;
         this.path = path;
         this.timestamp = timestamp;

src/main/java/org/codelibs/fess/exception/InvalidQueryException.java

@@ -22,7 +22,7 @@ public class InvalidQueryException extends FessSystemException {
 
     private static final long serialVersionUID = 1L;
 
-    private transient final VaMessenger<FessMessages> messageCode;
+    private final transient VaMessenger<FessMessages> messageCode;
 
     public InvalidQueryException(final VaMessenger<FessMessages> messageCode, final String message, final Throwable cause) {
         super(message, cause);

src/main/java/org/codelibs/fess/exception/SsoMessageException.java

@@ -22,7 +22,7 @@ public class SsoMessageException extends FessSystemException {
 
     private static final long serialVersionUID = 1L;
 
-    private transient final VaMessenger<FessMessages> messageCode;
+    private final transient VaMessenger<FessMessages> messageCode;
 
     public SsoMessageException(final VaMessenger<FessMessages> messageCode, final String message, final Throwable cause) {
         super(message, cause);

src/main/java/org/codelibs/fess/exec/Crawler.java

@@ -377,7 +377,9 @@ public class Crawler {
                 }
                 buf.append(entry.getKey()).append('=').append(entry.getValue());
             }
-            logger.info("[CRAWL INFO] {}", buf.toString());
+            if (logger.isInfoEnabled()) {
+                logger.info("[CRAWL INFO] {}", buf);
+            }
 
             // notification
             try {

src/main/java/org/codelibs/fess/job/PurgeDocJob.java

@@ -39,7 +39,7 @@ public class PurgeDocJob {
             searchEngineClient.deleteByQuery(fessConfig.getIndexDocumentUpdateIndex(), queryBuilder);
 
         } catch (final Exception e) {
-            logger.error("Could not delete expired documents: {}", queryBuilder.toString(), e);
+            logger.error("Could not delete expired documents: {}", queryBuilder, e);
             resultBuf.append(e.getMessage()).append("\n");
         }
 

src/main/java/org/codelibs/fess/ldap/LdapManager.java

@@ -139,7 +139,7 @@ public class LdapManager {
             try (DirContextHolder holder = getDirContext(() -> env)) {
                 final DirContext context = holder.get();
                 if (logger.isDebugEnabled()) {
-                    logger.debug("Logged in as Bind DN.", context);
+                    logger.debug("Logged in as Bind DN. {}", context);
                 }
                 isBind = true;
             } catch (final Exception e) {
@@ -186,7 +186,7 @@ public class LdapManager {
                 return OptionalEntity.empty();
             }
             if (logger.isDebugEnabled()) {
-                logger.debug("Logged in.", context);
+                logger.debug("Logged in. {}", context);
             }
             return OptionalEntity.of(ldapUser);
         } catch (final Exception e) {

src/main/java/org/codelibs/fess/query/TermQueryCommand.java

@@ -138,10 +138,13 @@ public class TermQueryCommand extends QueryCommand {
                         messages -> messages.addErrorsInvalidQueryUnsupportedSortField(UserMessages.GLOBAL_PROPERTY_KEY, sortField),
                         "Unsupported sort field: " + termQuery);
             }
-            SortOrder sortOrder;
+            final SortOrder sortOrder;
             if (values.length == 2) {
-                sortOrder = SortOrder.DESC.toString().equalsIgnoreCase(values[1]) ? SortOrder.DESC : SortOrder.ASC;
-                if (sortOrder == null) {
+                if (SortOrder.DESC.toString().equalsIgnoreCase(values[1])) {
+                    sortOrder = SortOrder.DESC;
+                } else if (SortOrder.ASC.toString().equalsIgnoreCase(values[1])) {
+                    sortOrder = SortOrder.ASC;
+                } else {
                     throw new InvalidQueryException(
                             messages -> messages.addErrorsInvalidQueryUnsupportedSortOrder(UserMessages.GLOBAL_PROPERTY_KEY, values[1]),
                             "Invalid sort order: " + termQuery);

src/main/java/org/codelibs/fess/sso/oic/OpenIdConnectAuthenticator.java

@@ -211,6 +211,8 @@ public class OpenIdConnectAuthenticator implements SsoAuthenticator {
                         }
                         attributes.put("groups", list.toArray(new String[list.size()]));
                         break;
+                    default:
+                        break;
                     }
                 }
             }

src/main/java/org/codelibs/fess/timer/HotThreadMonitorTarget.java

@@ -64,7 +64,9 @@ public class HotThreadMonitorTarget extends MonitorTarget {
         appendTimestamp(buf);
         buf.append('}');
 
-        logger.info(buf.toString());
+        if (logger.isInfoEnabled()) {
+            logger.info(buf.toString());
+        }
     }
 
 }

src/main/java/org/codelibs/fess/timer/SystemMonitorTarget.java

@@ -59,7 +59,9 @@ public class SystemMonitorTarget extends MonitorTarget {
         appendTimestamp(buf);
         buf.append('}');
 
-        logger.info(buf.toString());
+        if (logger.isInfoEnabled()) {
+            logger.info(buf.toString());
+        }
     }
 
     private void appendJvmStats(final StringBuilder buf) {

src/main/java/org/codelibs/fess/tomcat/webresources/FessWebResourceRoot.java

@@ -51,7 +51,10 @@ public class FessWebResourceRoot extends StandardRoot {
                         }
                     }
                 } catch (final Exception e) {
-                    logger.log(Level.WARNING, "Failed to read " + possibleJar, e);
+                    logger.log(Level.WARNING, e, () -> {
+                        final String canonicalPath = possibleJar.getCanonicalPath();
+                        return "Failed to read " + canonicalPath;
+                    });
                 }
             }
         }

src/test/java/org/codelibs/fess/helper/QueryHelperTest.java

@@ -317,10 +317,12 @@ public class QueryHelperTest extends UnitFessTestCase {
                 "{\"timestamp\":{\"order\":\"desc\"}}{\"last_modified\":{\"order\":\"desc\"}}", //
                 buildQuery("sort:timestamp.desc sort:last_modified.desc"));
 
-        assertQueryContext(query, Map.of(), //
-                Set.of(), //
-                "{\"timestamp\":{\"order\":\"asc\"}}", //
-                buildQuery("sort:timestamp.xxx"));
+        try {
+            buildQuery("sort:timestamp.xxx");
+            fail();
+        } catch (InvalidQueryException e) {
+            // ok
+        }
 
         try {
             buildQuery("sort:aaa");