From e538e5003bb80516b3d634ba50d6828e57867292 Mon Sep 17 00:00:00 2001 From: Mikhail_Navitski Date: Mon, 25 Mar 2024 21:35:45 +0100 Subject: [PATCH] Code optimization: convert class fields to local variables where it's possible --- CHANGELOG.md | 1 + .../impl/ErrorPageHandlerImpl.java | 6 ++--- .../config/impl/HttpCacheConfigImpl.java | 6 ++--- .../store/mem/impl/MemHttpCacheStoreImpl.java | 4 +-- .../commons/reports/models/ReportRunner.java | 9 +++---- .../twitter/impl/TwitterAdapterFactory.java | 16 +++--------- .../version/impl/CurrentEvolutionImpl.java | 10 +++----- .../version/impl/EvolutionAnalyserImpl.java | 6 ++--- .../version/impl/EvolutionContextImpl.java | 25 ++++++++----------- .../impl/lines/LinesGenerator.java | 19 ++++++-------- .../impl/AemEnvironmentIndicatorFilter.java | 22 ++++------------ 11 files changed, 41 insertions(+), 83 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22f65f3ace..3a580ca2b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com) - #3294 - Cloud manager report issues partial fix - #3295 - Updated the annotations in QueryReportConfig fixing the query manager issue due to empty query language - #3284 - Allow anonymous to read redirect caconfig options +- #2854 - Code optimization: convert class fields to local variables ## 6.4.0 - 2024-02-22 diff --git a/bundle/src/main/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImpl.java b/bundle/src/main/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImpl.java index ab37e57940..d2cafc98c3 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImpl.java +++ b/bundle/src/main/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImpl.java @@ -126,8 +126,6 @@ public final class ErrorPageHandlerImpl implements ErrorPageHandlerService { /* Fallback Error Code Extension */ private static final String DEFAULT_FALLBACK_ERROR_NAME = "500"; - private String fallbackErrorName = DEFAULT_FALLBACK_ERROR_NAME; - @Property( label = "Fallback error page name", description = "Error page name (not path) to use if a valid Error Code/Error Servlet Name cannot be " @@ -852,7 +850,7 @@ private void configure(ComponentContext componentContext) { PropertiesUtil.toString(config.get(legacyPrefix + PROP_ERROR_PAGE_EXTENSION), DEFAULT_ERROR_PAGE_EXTENSION)); - this.fallbackErrorName = PropertiesUtil.toString(config.get(PROP_FALLBACK_ERROR_NAME), + final String fallbackErrorName = PropertiesUtil.toString(config.get(PROP_FALLBACK_ERROR_NAME), PropertiesUtil.toString(config.get(legacyPrefix + PROP_FALLBACK_ERROR_NAME), DEFAULT_FALLBACK_ERROR_NAME)); @@ -936,7 +934,7 @@ private void configure(ComponentContext componentContext) { pw.printf("Enabled: %s", this.enabled).println(); pw.printf("System Error Page Path: %s", this.systemErrorPagePath).println(); pw.printf("Error Page Extension: %s", this.errorPageExtension).println(); - pw.printf("Fallback Error Page Name: %s", this.fallbackErrorName).println(); + pw.printf("Fallback Error Page Name: %s", fallbackErrorName).println(); pw.printf("Resource Not Found - Behavior: %s", this.notFoundBehavior).println(); pw.printf("Resource Not Found - Exclusion Path Patterns %s", Arrays.toString(tmpNotFoundExclusionPatterns)).println(); diff --git a/bundle/src/main/java/com/adobe/acs/commons/httpcache/config/impl/HttpCacheConfigImpl.java b/bundle/src/main/java/com/adobe/acs/commons/httpcache/config/impl/HttpCacheConfigImpl.java index cca0454d99..196af27919 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/httpcache/config/impl/HttpCacheConfigImpl.java +++ b/bundle/src/main/java/com/adobe/acs/commons/httpcache/config/impl/HttpCacheConfigImpl.java @@ -108,7 +108,6 @@ public class HttpCacheConfigImpl implements HttpCacheConfig { cardinality = Integer.MAX_VALUE) static final String PROP_BLACKLISTED_REQUEST_URI_PATTERNS = "httpcache.config.requesturi.patterns.blacklisted"; - private List blacklistedRequestUriPatterns; private List blacklistedRequestUriPatternsAsRegEx = Collections.emptyList(); // Authentication requirement @@ -136,7 +135,6 @@ public class HttpCacheConfigImpl implements HttpCacheConfig { + ". This accepts " + "REGEX. Example - /etc/my-products(.*)", cardinality = Integer.MAX_VALUE) static final String PROP_CACHE_INVALIDATION_PATH_PATTERNS = "httpcache.config.invalidation.oak.paths"; - private List cacheInvalidationPathPatterns; private List cacheInvalidationPathPatternsAsRegEx = Collections.emptyList(); // Cache store @@ -270,7 +268,7 @@ protected void activate(Map configs) { excludedCookieKeys = Arrays.asList(PropertiesUtil.toStringArray(configs.get(PROP_RESPONSE_COOKIE_KEY_EXCLUSIONS), new String[]{})); // Request URIs - Blacklisted. - blacklistedRequestUriPatterns = Arrays.asList(PropertiesUtil.toStringArray(configs + List blacklistedRequestUriPatterns = Arrays.asList(PropertiesUtil.toStringArray(configs .get(PROP_BLACKLISTED_REQUEST_URI_PATTERNS), new String[]{})); blacklistedRequestUriPatternsAsRegEx = compileToPatterns(blacklistedRequestUriPatterns); @@ -287,7 +285,7 @@ protected void activate(Map configs) { expiryOnUpdate = PropertiesUtil.toLong(configs.get(PROP_EXPIRY_ON_UPDATE), DEFAULT_EXPIRY_ON_UPDATE); // Cache invalidation paths. - cacheInvalidationPathPatterns = Arrays.asList(PropertiesUtil.toStringArray(configs + List cacheInvalidationPathPatterns = Arrays.asList(PropertiesUtil.toStringArray(configs .get(PROP_CACHE_INVALIDATION_PATH_PATTERNS), new String[]{})); cacheInvalidationPathPatternsAsRegEx = compileToPatterns(cacheInvalidationPathPatterns); diff --git a/bundle/src/main/java/com/adobe/acs/commons/httpcache/store/mem/impl/MemHttpCacheStoreImpl.java b/bundle/src/main/java/com/adobe/acs/commons/httpcache/store/mem/impl/MemHttpCacheStoreImpl.java index 0298a4267d..dbd07cf9c0 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/httpcache/store/mem/impl/MemHttpCacheStoreImpl.java +++ b/bundle/src/main/java/com/adobe/acs/commons/httpcache/store/mem/impl/MemHttpCacheStoreImpl.java @@ -94,8 +94,6 @@ public class MemHttpCacheStoreImpl extends AbstractGuavaCacheMBean cache; @@ -103,7 +101,7 @@ public class MemHttpCacheStoreImpl extends AbstractGuavaCacheMBean configs) { // Read config and populate values. ttl = PropertiesUtil.toLong(configs.get(PROP_TTL), DEFAULT_TTL); - maxSizeInMb = PropertiesUtil.toLong(configs.get(PROP_MAX_SIZE_IN_MB), DEFAULT_MAX_SIZE_IN_MB); + long maxSizeInMb = PropertiesUtil.toLong(configs.get(PROP_MAX_SIZE_IN_MB), DEFAULT_MAX_SIZE_IN_MB); // Initializing the cache. // If cache is present, invalidate all and reinitailize the cache. diff --git a/bundle/src/main/java/com/adobe/acs/commons/reports/models/ReportRunner.java b/bundle/src/main/java/com/adobe/acs/commons/reports/models/ReportRunner.java index c5a21092a4..1fe790d85d 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/reports/models/ReportRunner.java +++ b/bundle/src/main/java/com/adobe/acs/commons/reports/models/ReportRunner.java @@ -45,8 +45,6 @@ public class ReportRunner { private String failureMessage; - private int page; - private ReportExecutor reportExecutor; private SlingHttpServletRequest request; @@ -71,7 +69,7 @@ public ReportRunner(SlingHttpServletRequest request) { } @SuppressWarnings("squid:S2658") // class name is from a trusted source - private boolean executeConfig(Resource config, SlingHttpServletRequest request) { + private boolean executeConfig(Resource config, SlingHttpServletRequest request, int page) { log.trace("executeConfig"); try { Class exClass = ReportExecutorProvider.INSTANCE.getReportExecutor(dynamicClassLoaderManager, config); @@ -79,7 +77,7 @@ private boolean executeConfig(Resource config, SlingHttpServletRequest request) if (model instanceof ReportExecutor) { ReportExecutor ex = (ReportExecutor) model; ex.setConfiguration(config); - ex.setPage(this.page); + ex.setPage(page); this.reportExecutor = ex; return true; } else { @@ -110,6 +108,7 @@ public ReportExecutor getReportExecutor() { protected void init() { log.trace("init"); + int page; try { page = Integer.parseInt(request.getParameter("page"), 10); } catch (Exception e) { @@ -123,7 +122,7 @@ protected void init() { Iterator children = configCtr.listChildren(); while (children.hasNext()) { Resource config = children.next(); - if (executeConfig(config, request)) { + if (executeConfig(config, request, page)) { log.debug("Successfully executed report with configuration: {}", config); resultsRetrieved = true; break; diff --git a/bundle/src/main/java/com/adobe/acs/commons/twitter/impl/TwitterAdapterFactory.java b/bundle/src/main/java/com/adobe/acs/commons/twitter/impl/TwitterAdapterFactory.java index 35b83e5932..05bba39937 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/twitter/impl/TwitterAdapterFactory.java +++ b/bundle/src/main/java/com/adobe/acs/commons/twitter/impl/TwitterAdapterFactory.java @@ -41,10 +41,9 @@ class TwitterAdapterFactory implements AdapterFactory { TwitterAdapterFactory(String httpProxyHost, int httpProxyPort, boolean useSsl) { - this.httpProxyHost = httpProxyHost; - this.httpProxyPort = httpProxyPort; - this.useSsl = useSsl; - this.factory = new TwitterFactory(buildConfiguration()); + this.factory = new TwitterFactory( + buildConfiguration(httpProxyHost, httpProxyPort, useSsl) + ); } @VisibleForTesting @@ -58,13 +57,6 @@ class TwitterAdapterFactory implements AdapterFactory { private final TwitterFactory factory; - private final String httpProxyHost; - - private final int httpProxyPort; - - private final boolean useSsl; - - @SuppressWarnings("unchecked") @Override @@ -87,7 +79,7 @@ public AdapterType getAdapter(Object adaptable, Class return null; } - private Configuration buildConfiguration() { + private Configuration buildConfiguration(String httpProxyHost, int httpProxyPort, boolean useSsl) { final ConfigurationBuilder builder = new ConfigurationBuilder(); builder.setUseSSL(useSsl); builder.setJSONStoreEnabled(true); diff --git a/bundle/src/main/java/com/adobe/acs/commons/version/impl/CurrentEvolutionImpl.java b/bundle/src/main/java/com/adobe/acs/commons/version/impl/CurrentEvolutionImpl.java index f427836f30..b04821e0e0 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/version/impl/CurrentEvolutionImpl.java +++ b/bundle/src/main/java/com/adobe/acs/commons/version/impl/CurrentEvolutionImpl.java @@ -40,15 +40,11 @@ public class CurrentEvolutionImpl implements Evolution { private static final Logger log = LoggerFactory.getLogger(CurrentEvolutionImpl.class); - private final Resource resource; private final List versionEntries = new ArrayList(); - private EvolutionConfig config; public CurrentEvolutionImpl(Resource resource, EvolutionConfig config) { - this.resource = resource; - this.config = config; try { - populate(this.resource, 0); + populate(resource, config, 0); } catch (RepositoryException e) { log.warn("Could not populate Evolution", e); } @@ -74,7 +70,7 @@ public List getVersionEntries() { return Collections.unmodifiableList(this.versionEntries); } - private void populate(Resource r, int depth) throws PathNotFoundException, RepositoryException { + private void populate(Resource r, EvolutionConfig config, int depth) throws PathNotFoundException, RepositoryException { ValueMap map = r.getValueMap(); List keys = new ArrayList(map.keySet()); Collections.sort(keys); @@ -92,7 +88,7 @@ private void populate(Resource r, int depth) throws PathNotFoundException, Repos String relPath = EvolutionPathUtil.getLastRelativeResourceName(child.getPath()); if (config.handleResource(relPath)) { versionEntries.add(new CurrentEvolutionEntryImpl(child, config)); - populate(child, depth); + populate(child, config, depth); } depth--; } diff --git a/bundle/src/main/java/com/adobe/acs/commons/version/impl/EvolutionAnalyserImpl.java b/bundle/src/main/java/com/adobe/acs/commons/version/impl/EvolutionAnalyserImpl.java index 6a9bf78bb0..dbfd02b911 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/version/impl/EvolutionAnalyserImpl.java +++ b/bundle/src/main/java/com/adobe/acs/commons/version/impl/EvolutionAnalyserImpl.java @@ -47,8 +47,6 @@ public class EvolutionAnalyserImpl implements EvolutionAnalyser { protected static final String PROPERTY_IGNORES = "properties.ignore"; protected static final String RESOURCE_IGNORES = "resources.ignore"; - private String[] propertyIgnores; - private String[] resourceIgnores; private EvolutionConfig evolutionConfig; @@ -59,8 +57,8 @@ public EvolutionContext getEvolutionContext(Resource resource) { @Activate protected void activate(final Map config) { - propertyIgnores = PropertiesUtil.toStringArray(config.get(PROPERTY_IGNORES), new String[] { "" }); - resourceIgnores = PropertiesUtil.toStringArray(config.get(RESOURCE_IGNORES), new String[] { "" }); + String[] propertyIgnores = PropertiesUtil.toStringArray(config.get(PROPERTY_IGNORES), new String[]{""}); + String[] resourceIgnores = PropertiesUtil.toStringArray(config.get(RESOURCE_IGNORES), new String[]{""}); evolutionConfig = new EvolutionConfig(propertyIgnores, resourceIgnores); log.debug("Ignored properties: {}", (Object[]) propertyIgnores); log.debug("Ignored resources: {}", (Object[]) resourceIgnores); diff --git a/bundle/src/main/java/com/adobe/acs/commons/version/impl/EvolutionContextImpl.java b/bundle/src/main/java/com/adobe/acs/commons/version/impl/EvolutionContextImpl.java index 85ed36e696..738eb02511 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/version/impl/EvolutionContextImpl.java +++ b/bundle/src/main/java/com/adobe/acs/commons/version/impl/EvolutionContextImpl.java @@ -37,18 +37,14 @@ public class EvolutionContextImpl implements EvolutionContext { private static final Logger log = LoggerFactory.getLogger(EvolutionContext.class); - private Resource resource = null; - private VersionHistory history = null; - private ResourceResolver resolver = null; - private VersionManager versionManager = null; private List versions = new ArrayList(); private List evolutionItems = new ArrayList(); - private EvolutionConfig config; - public EvolutionContextImpl(Resource resource, EvolutionConfig config) { - this.resource = resource.isResourceType("cq:Page") ? resource.getChild("jcr:content") : resource; - this.config = config; - populateEvolutions(); + public EvolutionContextImpl(Resource providedResource, EvolutionConfig config) { + Resource resource = providedResource.isResourceType("cq:Page") + ? providedResource.getChild("jcr:content") + : providedResource; + populateEvolutions(resource, config); } @Override @@ -61,11 +57,11 @@ public List getVersions() { return Collections.unmodifiableList(versions); } - private void populateEvolutions() { + private void populateEvolutions(Resource resource, EvolutionConfig config) { try { - this.resolver = resource.getResourceResolver(); - this.versionManager = resolver.adaptTo(Session.class).getWorkspace().getVersionManager(); - this.history = versionManager.getVersionHistory(resource.getPath()); + ResourceResolver resolver = resource.getResourceResolver(); + VersionManager versionManager = resolver.adaptTo(Session.class).getWorkspace().getVersionManager(); + VersionHistory history = versionManager.getVersionHistory(resource.getPath()); Iterator iter = history.getAllVersions(); while (iter.hasNext()) { Version next = iter.next(); @@ -80,7 +76,6 @@ private void populateEvolutions() { log.error("Could not find versions", e); } evolutionItems = new ArrayList<>(versions); - evolutionItems.add(new CurrentEvolutionImpl(this.resource, this.config)); + evolutionItems.add(new CurrentEvolutionImpl(resource, config)); } - } diff --git a/bundle/src/main/java/com/adobe/acs/commons/wcm/comparisons/impl/lines/LinesGenerator.java b/bundle/src/main/java/com/adobe/acs/commons/wcm/comparisons/impl/lines/LinesGenerator.java index 340e1805ac..d7ecf858f2 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/wcm/comparisons/impl/lines/LinesGenerator.java +++ b/bundle/src/main/java/com/adobe/acs/commons/wcm/comparisons/impl/lines/LinesGenerator.java @@ -45,10 +45,8 @@ public class LinesGenerator { private Stepper right; private T leftValue; - private int leftSpacer; private T rightValue; - private int rightSpacer; public LinesGenerator(Function toId) { this.toId = toId; @@ -64,24 +62,23 @@ public List> generate(final Iterable left, Iterable right) { this.rightValue = this.right.next(); do { - this.leftSpacer = this.right.positionOfIdAfterCurrent(leftValue); - this.rightSpacer = this.left.positionOfIdAfterCurrent(rightValue); + int leftSpacer = this.right.positionOfIdAfterCurrent(leftValue); + int rightSpacer = this.left.positionOfIdAfterCurrent(rightValue); if (leftValue != null && rightValue != null && toId.apply(leftValue).equals(toId.apply(rightValue))) { addPair(lines); } else if (leftSpacer < rightSpacer && leftSpacer > 0) { - addWithLeftSpacers(lines); + addWithLeftSpacers(lines, leftSpacer); } else if (rightSpacer > 0) { - addWithRightSpacers(lines); + addWithRightSpacers(lines, rightSpacer); } else if (leftSpacer > 0) { - addWithLeftSpacers(lines); + addWithLeftSpacers(lines, leftSpacer); } else { addSeperated(lines); - } } while (leftValue != null || rightValue != null); @@ -99,14 +96,14 @@ private void addSeperated(List> lines) { } } - private void addWithLeftSpacers(List> lines) { + private void addWithLeftSpacers(List> lines, int leftSpacer) { for (int i = 0; i < leftSpacer; i++) { lines.add(LineImpl.right(rightValue)); rightValue = this.right.next(); } } - private void addWithRightSpacers(List> lines) { + private void addWithRightSpacers(List> lines, int rightSpacer) { for (int i = 0; i < rightSpacer; i++) { lines.add(LineImpl.left(leftValue)); leftValue = this.left.next(); @@ -118,6 +115,4 @@ private void addPair(List> lines) { this.leftValue = this.left.next(); this.rightValue = this.right.next(); } - - } diff --git a/bundle/src/main/java/com/adobe/acs/commons/wcm/impl/AemEnvironmentIndicatorFilter.java b/bundle/src/main/java/com/adobe/acs/commons/wcm/impl/AemEnvironmentIndicatorFilter.java index b4e5110372..76bbb840ce 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/wcm/impl/AemEnvironmentIndicatorFilter.java +++ b/bundle/src/main/java/com/adobe/acs/commons/wcm/impl/AemEnvironmentIndicatorFilter.java @@ -103,8 +103,6 @@ public class AemEnvironmentIndicatorFilter implements Filter { /* Property: Default Color */ - private String color = ""; - @Property(label = "Color", description = "The color of the indicator bar; takes any valid value" + " for CSS's 'background-color' attribute." @@ -114,8 +112,6 @@ public class AemEnvironmentIndicatorFilter implements Filter { /* Property: CSS Override */ - private String cssOverride = ""; - @Property(label = "CSS Override", description = "Accepts any valid CSS to style the AEM indicator div. All CSS rules must only be " + "scoped to #" + DIV_ID + " { .. }", @@ -141,8 +137,6 @@ public class AemEnvironmentIndicatorFilter implements Filter { /* Property: Always Include Base CSS */ - private boolean alwaysIncludeBaseCss; - @Property(label = "Always Include Base CSS", description = "Always include the base CSS scoped to #" + DIV_ID + " { .. }", boolValue = false) @@ -150,8 +144,6 @@ public class AemEnvironmentIndicatorFilter implements Filter { /* Property: Always Include Color CSS */ - private boolean alwaysIncludeColorCss; - @Property(label = "Always Include Color CSS", description = "Always include the color CSS scoped to #" + DIV_ID + " { .. }", boolValue = false) @@ -167,7 +159,6 @@ public class AemEnvironmentIndicatorFilter implements Filter { description = "Do not display the indicator when these WCM modes are active", cardinality = Integer.MAX_VALUE) public static final String PROP_EXCLUDED_WCMMODES = "excluded-wcm-modes"; - private String[] excludedWCMModes; private static final String[] DEFAULT_ALLOWED_EXTENSIONS = {"html", "htm", "jsp", NO_EXTENSION_PLACEHOLDER}; @@ -345,15 +336,12 @@ boolean isEditExperienceFragmentVariation(String headerValue, String requestUri) protected final void activate(ComponentContext ctx) { Dictionary config = ctx.getProperties(); - color = PropertiesUtil.toString(config.get(PROP_COLOR), ""); - cssOverride = PropertiesUtil.toString(config.get(PROP_CSS_OVERRIDE), ""); innerHTML = PropertiesUtil.toString(config.get(PROP_INNER_HTML), ""); innerHTML = new StrSubstitutor(StrLookup.systemPropertiesLookup()).replace(innerHTML); - alwaysIncludeBaseCss = PropertiesUtil.toBoolean(PROP_ALWAYS_INCLUDE_BASE_CSS, false); - alwaysIncludeColorCss = PropertiesUtil.toBoolean(PROP_ALWAYS_INCLUDE_COLOR_CSS, false); - - alwaysIncludeBaseCss = PropertiesUtil.toBoolean(PROP_ALWAYS_INCLUDE_BASE_CSS, false); - alwaysIncludeColorCss = PropertiesUtil.toBoolean(PROP_ALWAYS_INCLUDE_COLOR_CSS, false); + String color = PropertiesUtil.toString(config.get(PROP_COLOR), ""); + String cssOverride = PropertiesUtil.toString(config.get(PROP_CSS_OVERRIDE), ""); + boolean alwaysIncludeBaseCss = PropertiesUtil.toBoolean(PROP_ALWAYS_INCLUDE_BASE_CSS, false); + boolean alwaysIncludeColorCss = PropertiesUtil.toBoolean(PROP_ALWAYS_INCLUDE_COLOR_CSS, false); StringBuilder cssSb = new StringBuilder(); @@ -374,7 +362,7 @@ protected final void activate(ComponentContext ctx) { titlePrefix = xss.encodeForJSString( PropertiesUtil.toString(config.get(PROP_TITLE_PREFIX), "").toString()); - excludedWCMModes = PropertiesUtil.toStringArray(config.get(PROP_EXCLUDED_WCMMODES), + String[] excludedWCMModes = PropertiesUtil.toStringArray(config.get(PROP_EXCLUDED_WCMMODES), DEFAULT_EXCLUDED_WCMMODES); allowedExtensions = PropertiesUtil.toStringArray(config.get(PROP_ALLOWED_EXTENSIONS),