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

MapDotKeyReplacer throws an ConcurrentModificationException #910

Open
luca010 opened this issue Nov 5, 2019 · 6 comments
Open

MapDotKeyReplacer throws an ConcurrentModificationException #910

luca010 opened this issue Nov 5, 2019 · 6 comments
Labels

Comments

@luca010
Copy link
Contributor

@luca010 luca010 commented Nov 5, 2019

MapKeyDotReplacer throws an ConcurrentModificationException when tries to replace dots in the document Map. I would suggest to rewrite the method replaceInMapKeys in the MapKeyDotReplacer class as follows:

private Document replaceInMapKeys(Document map, String regexFrom, String from, String to) {
    Document result = new Document();
    for (String key : map.keySet()) {
        Object val = map.get(key);
        if (key.contains(from)) {
            String escaped = key.replaceAll(regexFrom, to);
            result.put(escaped, val);
        } else {
           result.put(key, val);
        }
    }
    return result;
}
@bartoszwalacik

This comment has been minimized.

Copy link
Member

@bartoszwalacik bartoszwalacik commented Nov 5, 2019

Hi @luca010 , please provide a Runnable test case which isolates the bug and allows us to easily reproduce it on our laptops. You can push this test case to your fork of this repository.

luca010 added a commit to luca010/javers that referenced this issue Nov 5, 2019
@luca010

This comment has been minimized.

Copy link
Contributor Author

@luca010 luca010 commented Nov 5, 2019

Hi @bartoszwalacik , I forked this repository and I added a new test case to reproduce the bug. I hope this will help you.

@bartoszwalacik

This comment has been minimized.

Copy link
Member

@bartoszwalacik bartoszwalacik commented Nov 6, 2019

Great, so now it shuld be easy for you to contribute the fix :)

luca010 added a commit to luca010/javers that referenced this issue Nov 6, 2019
@luca010

This comment has been minimized.

Copy link
Contributor Author

@luca010 luca010 commented Nov 6, 2019

I pushed the fix on my fork. I run all the test cases with success.

@bartoszwalacik

This comment has been minimized.

Copy link
Member

@bartoszwalacik bartoszwalacik commented Nov 7, 2019

ok, please create the pull request from your fork to javers master

@bartoszwalacik

This comment has been minimized.

Copy link
Member

@bartoszwalacik bartoszwalacik commented Nov 11, 2019

fix is released in 5.8.5
@luca010 thanks for your contribution, well done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.