You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2022/05/23 10:05:58 UTC
[GitHub] [sling-org-apache-sling-rewriter] kwin commented on a diff in pull request #7: SLING-11317 Reworked ProcessorManagerImpl, added Tests
kwin commented on code in PR #7:
URL: https://github.com/apache/sling-org-apache-sling-rewriter/pull/7#discussion_r879263405
##########
src/main/java/org/apache/sling/rewriter/impl/ProcessorManagerImpl.java:
##########
@@ -240,6 +240,7 @@ protected void addProcessor(final String key, final String configPath, final Pro
ConfigEntry[] newConfigs = new ConfigEntry[configs.length + 1];
System.arraycopy(configs, 0, newConfigs, 0, configs.length);
newConfigs[configs.length] = new ConfigEntry(configPath, config);
+ configs=newConfigs;
Review Comment:
```suggestion
configs = newConfigs;
```
##########
src/main/java/org/apache/sling/rewriter/impl/ProcessorManagerImpl.java:
##########
@@ -299,13 +300,11 @@ public synchronized void printConfiguration(final PrintWriter pw) {
private synchronized void updateProcessor(final String path) {
final int pos = path.lastIndexOf('/');
final String key = path.substring(pos + 1);
- int keyIndex = 0;
// search the search path
for(final String sp : this.searchPath) {
Review Comment:
This loop doesn't seem to do anything any longer, right? Can't we remove it?
##########
src/main/java/org/apache/sling/rewriter/impl/ProcessorManagerImpl.java:
##########
@@ -327,61 +326,14 @@ private synchronized void updateProcessor(final String path) {
}
if ( index != -1 ) {
// we are already in the array
- if ( index == 0 ) {
- // we are the first, so remove the old, and add the new
this.orderedProcessors.remove(configs[index].config);
Review Comment:
I would remove this optimization as well and would just call `addProcessor` in all cases, that simplifies the code and should be neglectable from a performance PoV
##########
src/main/java/org/apache/sling/rewriter/impl/ProcessorManagerImpl.java:
##########
@@ -327,61 +326,14 @@ private synchronized void updateProcessor(final String path) {
}
if ( index != -1 ) {
// we are already in the array
- if ( index == 0 ) {
- // we are the first, so remove the old, and add the new
this.orderedProcessors.remove(configs[index].config);
configs[index] = new ConfigEntry(path, config);
if ( config.isActive() ) {
this.orderedProcessors.add(config);
Collections.sort(this.orderedProcessors, new ProcessorConfiguratorComparator());
- }
- } else {
- // we are not the first, so we can simply exchange
- configs[index] = new ConfigEntry(path, config);
}
} else {
- // now we have to insert the new config at the correct place
- int insertIndex = 0;
- boolean found = false;
- while ( !found && insertIndex < configs.length) {
- final ConfigEntry current = configs[insertIndex];
- int currentIndex = -1;
- for(int i=0; i<searchPath.length; i++) {
- if ( current.path.startsWith(searchPath[i]) ) {
- currentIndex = i;
- break;
- }
- }
- if ( currentIndex >= keyIndex ) {
- found = true;
- insertIndex = currentIndex;
- }
- }
-
- if ( !found ) {
- // just append
- this.addProcessor(key, path, config);
- } else {
- ConfigEntry[] newArray = new ConfigEntry[configs.length + 1];
- int i = 0;
- for(final ConfigEntry current : configs) {
- if ( i == insertIndex ) {
- newArray[i] = new ConfigEntry(path, config);
- i++;
- }
- newArray[i] = current;
- i++;
- }
- this.processors.put(key, newArray);
- if ( insertIndex == 0 ) {
- // we are the first, so remove the old, and add the new
- this.orderedProcessors.remove(configs[1].config);
- if ( config.isActive() ) {
- this.orderedProcessors.add(config);
- Collections.sort(this.orderedProcessors, new ProcessorConfiguratorComparator());
- }
- }
- }
+ this.addProcessor(key, path, config);
Review Comment:
spaces instead of tab
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org