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

SLING-8523 : Duplicate OSGI configs after upgrade #4

Merged
merged 3 commits into from
Jun 24, 2019

Conversation

ashokpanghal
Copy link
Contributor

Upgrade resulting in duplicate factory configs for cases where alais is null

Upgrade resulting in duplicate factory configs for cases where alais is null
@ashokpanghal
Copy link
Contributor Author

@cziegeler , please review


final String factoryPid = alias.substring(0, pos - 1);
final String pid = oldId.substring(factoryPid.length() + 1);
String factoryIdString = oldId.substring(0,oldId.lastIndexOf('.')+1); // keep it +1 to have last dot intact so that we always have even dots in the string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could store lastIndexOf in an int variable and reuse it below

final String pid = oldId.substring(factoryPid.length() + 1);
String factoryIdString = oldId.substring(0,oldId.lastIndexOf('.')+1); // keep it +1 to have last dot intact so that we always have even dots in the string
factoryPid = oldId.substring(0,getMiddleDotSplitIndex(factoryIdString,'.'));
pid = oldId.substring(oldId.lastIndexOf('.')+1,oldId.length());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

substring(int) is enough, if you only specify the start it goes until the of the string

@ashokpanghal
Copy link
Contributor Author

@cziegeler , Thanks for the review, added a test case to cover the new code and incorporated your code comments. Please merge.
On a side note, sonar code analysis is still showing a failure despite 100% code coverage for newly added code

Added cleanup for existing duplicate configs generated before this Patch
Copy link
Contributor

@cziegeler cziegeler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updated patch, it looks good to me


return new String[] { factoryPid, pid };
}

private int getMiddleDotSplitIndex(final String strId, char dot){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is only invoked with the second param being a ".", so you only need one param and can use a constant for "." below

@cziegeler cziegeler merged commit 221646e into apache:master Jun 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants