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

Fix MongoState.delete(Boolean) #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

leonjoosse
Copy link

MongoState did not override AbstractState.destroy(Boolean). In AbstractState.destroy(Boolean) is the variable 'service' used, which is/can become null. The MongoState class uses primarily the 'provider' variable. This caused agents to be deleted from the InstantiationService, but not from the database.

Note: this should maybe be fixed in another way by calling getService() instead of using the 'provider' variable?

MongoState did not override AbstractState.destroy(Boolean). In AbstractState.destroy(Boolean) is the variable 'service' used, which is/can become null. The MongoState class uses primarily the 'provider' variable. This caused agents to be deleted from the InstantiationService, but not from the database.

Note: this should maybe be fixed in another way by calling getService() instead of using the 'provider' variable?
Copy link
Member

@ludost ludost left a comment

Choose a reason for hiding this comment

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

Hi Leon,
I don't really understand what this change fixes? Both provider and getService() lead to the same object and the same delete() method within that object. So, your change doesn't change anything?

@leonjoosse
Copy link
Author

Because AbstractState#delete(Boolean) uses the AbstractState#service directly. It doesn't call getService().
https://github.com/almende/eve-java/blob/master/states/eve_state/src/main/java/com/almende/eve/state/AbstractState.java#L288

@ludost
Copy link
Member

ludost commented Sep 29, 2016

That's exactly my point: AbstractState#service is pointing to the same object as MongoState@provider. As long as the pointer is not set to null (which the current code never does), your code doesn't change anything. So, my question remains: what are your trying to fix? What are the symptomes?

@leonjoosse
Copy link
Author

I also expected to happen what you day, but when stepping through the test with a debugger, AbstractState#service is null, while MongoState#provider is not null. I'll get back to you on how to reproduce it.

screenie-3

@ludost
Copy link
Member

ludost commented Sep 29, 2016

I noticed that MongoState also overrides setService(), setting the provider field, but not updating the parent object. This will lead to inconsistencies when the service is set later than during construction. Best to cleanup MongoState to directly use AbstractState#getService() instead of a local copy.

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