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

TRUNK-4140: Substitute deprecated method call in HibernateContextDAO #822

Closed
wants to merge 1 commit into from

Conversation

sashrika
Copy link
Contributor

removed "ConfigHelper.getResourceAsStream" deprecatede method.

added junit test "should_mergeDefaultRuntimeProperties()" in ContextDAOTest.java class.
But when testing it throws java.util.ConcurrentModificationException.
as i read in http://docs.oracle.com/javase/6/docs/api/java/util/ConcurrentModificationException.html and http://stackoverflow.com/questions/1931180/how-do-i-synchronize-to-prevent-a-java-util-concurrentmodificationexception , the reason is , we change a element in a java Collection while we iterate through it .

One possible solution would be using ConcurrentHashMap. But here we use java.util.Property.

The other possible solution would be instead of changing the values while iterating through a collection, cache the changes and then copy them to the Collection.

I have used the latter approach here.

@dkayiwa
Copy link
Member

dkayiwa commented Mar 26, 2014

@sashrika are all these changes just about replacing OpenmrsUtil.loadProperties() with the non deprecated one?

@dkayiwa
Copy link
Member

dkayiwa commented Mar 26, 2014

@sashrika i can reopen this after your response to the review comments.

@dkayiwa dkayiwa closed this Mar 26, 2014
@sashrika
Copy link
Contributor Author

@dkayiwa , Yes.
The problem comes when we going to write unit tests. When we invokde mergeDefaultRuntimeProperties(Properties runtimeProperties) in ContextDAOTest, it threw a ConcurrentModificationException. to get rid of that I had to do some changed in the code.

@sashrika
Copy link
Contributor Author

@dkayiwa , I romoved catching throwables.
Added catching runtime exceptions. I made a new commit and created a pull req
#824

@sashrika sashrika mentioned this pull request Mar 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants