Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code optimization: convert fields to local variables where it's possible #3302

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> blacklistedRequestUriPatterns;
private List<Pattern> blacklistedRequestUriPatternsAsRegEx = Collections.emptyList();

// Authentication requirement
Expand Down Expand Up @@ -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<String> cacheInvalidationPathPatterns;
private List<Pattern> cacheInvalidationPathPatternsAsRegEx = Collections.emptyList();

// Cache store
Expand Down Expand Up @@ -270,7 +268,7 @@ protected void activate(Map<String, Object> 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<String> blacklistedRequestUriPatterns = Arrays.asList(PropertiesUtil.toStringArray(configs
.get(PROP_BLACKLISTED_REQUEST_URI_PATTERNS), new String[]{}));
blacklistedRequestUriPatternsAsRegEx = compileToPatterns(blacklistedRequestUriPatterns);

Expand All @@ -287,7 +285,7 @@ protected void activate(Map<String, Object> configs) {
expiryOnUpdate = PropertiesUtil.toLong(configs.get(PROP_EXPIRY_ON_UPDATE), DEFAULT_EXPIRY_ON_UPDATE);

// Cache invalidation paths.
cacheInvalidationPathPatterns = Arrays.asList(PropertiesUtil.toStringArray(configs
List<String> cacheInvalidationPathPatterns = Arrays.asList(PropertiesUtil.toStringArray(configs
.get(PROP_CACHE_INVALIDATION_PATH_PATTERNS), new String[]{}));
cacheInvalidationPathPatternsAsRegEx = compileToPatterns(cacheInvalidationPathPatterns);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,14 @@ public class MemHttpCacheStoreImpl extends AbstractGuavaCacheMBean<CacheKey, Mem
private static final String PROP_MAX_SIZE_IN_MB = "httpcache.cachestore.memcache.maxsize";
private static final long DEFAULT_MAX_SIZE_IN_MB = 10L; // Defaults to 10MB.

private long maxSizeInMb;

/** Cache - Uses Google Guava's cache */
private Cache<CacheKey, MemCachePersistenceObject> cache;

@Activate
protected void activate(Map<String, Object> 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ public class ReportRunner {

private String failureMessage;

private int page;

private ReportExecutor reportExecutor;

private SlingHttpServletRequest request;
Expand All @@ -71,15 +69,15 @@ 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);
Object model = request.adaptTo(exClass);
if (model instanceof ReportExecutor) {
ReportExecutor ex = (ReportExecutor) model;
ex.setConfiguration(config);
ex.setPage(this.page);
ex.setPage(page);
this.reportExecutor = ex;
return true;
} else {
Expand Down Expand Up @@ -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) {
Expand All @@ -123,7 +122,7 @@ protected void init() {
Iterator<Resource> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -87,7 +79,7 @@ public <AdapterType> AdapterType getAdapter(Object adaptable, Class<AdapterType>
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<EvolutionEntry> versionEntries = new ArrayList<EvolutionEntry>();
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);
}
Expand All @@ -74,7 +70,7 @@ public List<EvolutionEntry> 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<String> keys = new ArrayList<String>(map.keySet());
Collections.sort(keys);
Expand All @@ -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--;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -59,8 +57,8 @@ public EvolutionContext getEvolutionContext(Resource resource) {

@Activate
protected void activate(final Map<String, String> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Evolution> versions = new ArrayList<Evolution>();
private List<Evolution> evolutionItems = new ArrayList<Evolution>();
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
Expand All @@ -61,11 +57,11 @@ public List<Evolution> 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<Version> iter = history.getAllVersions();
while (iter.hasNext()) {
Version next = iter.next();
Expand All @@ -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));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,8 @@ public class LinesGenerator<T> {
private Stepper<T> right;

private T leftValue;
private int leftSpacer;

private T rightValue;
private int rightSpacer;

public LinesGenerator(Function<T, Serializable> toId) {
this.toId = toId;
Expand All @@ -64,24 +62,23 @@ public List<Line<T>> generate(final Iterable<T> left, Iterable<T> 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);

Expand All @@ -99,14 +96,14 @@ private void addSeperated(List<Line<T>> lines) {
}
}

private void addWithLeftSpacers(List<Line<T>> lines) {
private void addWithLeftSpacers(List<Line<T>> lines, int leftSpacer) {
for (int i = 0; i < leftSpacer; i++) {
lines.add(LineImpl.right(rightValue));
rightValue = this.right.next();
}
}

private void addWithRightSpacers(List<Line<T>> lines) {
private void addWithRightSpacers(List<Line<T>> lines, int rightSpacer) {
for (int i = 0; i < rightSpacer; i++) {
lines.add(LineImpl.left(leftValue));
leftValue = this.left.next();
Expand All @@ -118,6 +115,4 @@ private void addPair(List<Line<T>> lines) {
this.leftValue = this.left.next();
this.rightValue = this.right.next();
}


}
Loading
Loading