search

multi-tenant / grails-multi-tenant-single-db

Multi Tenant implementation for single database environments - Use grails-filtering and grails-hibernate-hijacker
Apache License 2.0
32 stars 25 forks source link

Potential doWithTenantId Problem #20

Closed ericmartineau closed 12 years ago

ericmartineau commented 12 years ago

I'm not 100% I have the correct understanding here, but I think there is a problem with the method doWithTenantId()

Inside this method, the code changes the tenantId, runs some code, and then changes the tenantId to what it was before. The problem is that the transaction isn't actually committed until AFTER you set the tenantId back in the finally block.

Upon commit, all the hibernate operations will either fail (due to incorrect tenantIds), or worse yet, will save data into the wrong tenant state.

I think the correct behavior would be to move the currentTenant.set() method calls outside the hibernate.withSession block.

I don't mind making the change, but wanted to make sure I understood what was intended...

kimble commented 12 years ago

You're absolutely onto something there. I don't remember the details, but I think ran into some Hibernate trouble when I tried to implement this so I've just handled this manually the few places I've used this functionality in the application I worked on back then.

Feel free to commit the necessary changes!

basejump commented 12 years ago

Let me know if this looks right. This also fixed a failing test that I was having issues with. https://github.com/multi-tenant/grails-multi-tenant-single-db/commit/49a92e42b655a49be208e5f4e00fc4e6a82ad647

kimble commented 12 years ago

Looks good to me. Awesome to see commits being made to this project!