@Multitenant appears to cause hasErrors to always return false

[dalelotts]

I have a simple project using Grails 2.3.7 and this plugin. I have two identical tables, one with @MultiTenant and one without. Both tables have a name field with a unique constraint. name(nullable: false, blank: false, unique: 'tenantId') for the multi-tenant table.

Saving a new record that violates the unique name constraint for the multi-tenant table throws a DataIntegrityViolationException (as if failOnError:true), while the non-multi-tenant table behaves as expected ( .save returns null and .errors list is populated)

The controller checks .hasErrors() before calling .save() - the @MultiTenant table always returns false from hasErrors().

Is there something I have configured incorrectly? It does not seem like this plugin should impact the handling of validation.

[dalelotts]

It appears this plugin is dead - and not working correctly for me - is there an alternative approach that I can use?

[cogdachris]

@dalelotts I have not experienced this error and I have used the Multitenant plugin on a few projects.
Is your project on Github?

[scryan7371]

I use this plugin all the time and have not experienced any issues

Sent from my iPhone

On Mar 14, 2014, at 4:25 PM, cogdachris [email protected] wrote:

I have not experienced this error and I have used the Multitenant plugin on a few projects.

Is your project on Github?

—
Reply to this email directly or view it on GitHub.

[dalelotts]

It's probably something I am doing wrong - I will post a simple project on github tomorrow it would be great if someone would be willing to have a look and let me know what I am doing wrong.

[dalelotts]

It was super simple to re-create - Please see https://github.com/dalelotts/MultiTenantTest

@scryan7371, @cogdachris, @kimble, or @burtbeckwith, are you willing to have a look and let me know what I can do to correct this issue?

[sronderos]

Hi Dale,

Sorry for the delayed response. I think you have found a legitimate bug here. I was suspicious at 1st because I use this plugin in a production app and I use the unique: 'tenantId' feature the same way you've done in your example. After banging my head against this for awhile I looked at my live app and found this:

        // hack: setting tenantId manually to avoid validation error
        customer.tenantId = springSecurityService.currentUser?.userTenantId

Embarrassing...

I believe the issue is that GORM validation occurs before the mt plugin (which uses Hibernate Filters) updates the tenantId of the to-be-saved object. GORM validation passes because the tenantId is undefined during validation, then the Hibernate Filters apply the real tenantId and the DB throws that constraint violation, which is what you are seeing.

Long story short, it is a bug. You can work around it by setting the tenantId before calling .hasErrors() or .save() on any @MultiTenant domain objects. I know that the workaround is not ideal. I'm not sure if I will have time to figure out a fix for the underlying problem anytime soon.

Hope that helps,
Steve

[dalelotts]

Hi Steve,

Thanks for having a look at this! Now I feel like I can move forward, so that's good news!

The not-so-good news is that I have 40+ @MultiTenant domain objects. =( This may be a crazy idea, but what if the plug-in would override save() and validate() to set the tenantId?

Thanks again for the workaround!

[scryan7371]

We just use a custom validator in our domain base class and annotate all the fields that should be unique by tenanted and it seems to work fine

Sent from my iPhone

On Mar 15, 2014, at 2:54 PM, Steve Ronderos [email protected] wrote:

Hi Dale,

Sorry for the delayed response. I think you have found a legitimate bug here. I was suspicious at 1st because I use this plugin in a production app and I use the unique: 'tenantId' feature the same way you've done in your example. After banging my head against this for awhile I looked at my live app and found this:

    // hack: setting tenantId manually to avoid validation error
    customer.tenantId = springSecurityService.currentUser?.userTenantId

Embarrassing...

I believe the issue is that GORM validation occurs before the mt plugin (which uses Hibernate Filters) updates the tenantId of the to-be-saved object. GORM validation passes because the tenantId is undefined during validation, then the Hibernate Filters apply the real tenantId and the DB throws that constraint violation, which is what you are seeing.

Long story short, it is a bug. You can work around it by setting the tenantId before calling .hasErrors() or .save() on any @MultiTenant domain objects. I know that the workaround is not ideal. I'm not sure if I will have time to figure out a fix for the underlying problem anytime soon.

Hope that helps,
Steve

—
Reply to this email directly or view it on GitHub.

[lucastex]

Me too.

Custom validators worked great instead of the workaround and using the
Munique constraint. Try that :)
On Mar 15, 2014 10:33 PM, "Scott Ryan" [email protected] wrote:

We just use a custom validator in our domain base class and annotate all
the fields that should be unique by tenanted and it seems to work fine

Sent from my iPhone

On Mar 15, 2014, at 2:54 PM, Steve Ronderos [email protected]
wrote:

Hi Dale,

Sorry for the delayed response. I think you have found a legitimate bug
here. I was suspicious at 1st because I use this plugin in a production app
and I use the unique: 'tenantId' feature the same way you've done in your
example. After banging my head against this for awhile I looked at my live
app and found this:

// hack: setting tenantId manually to avoid validation error
customer.tenantId = springSecurityService.currentUser?.userTenantId
Embarrassing...

I believe the issue is that GORM validation occurs before the mt plugin
(which uses Hibernate Filters) updates the tenantId of the to-be-saved
object. GORM validation passes because the tenantId is undefined during
validation, then the Hibernate Filters apply the real tenantId and the DB
throws that constraint violation, which is what you are seeing.

Long story short, it is a bug. You can work around it by setting the
tenantId before calling .hasErrors() or .save() on any @MultiTenant domain
objects. I know that the workaround is not ideal. I'm not sure if I will
have time to figure out a fix for the underlying problem anytime soon.

Hope that helps,
Steve

Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHubhttps://github.com//issues/37#issuecomment-37745168
.

[dalelotts]

@scryan7371 and @lucastex I'm not completely sure what you are describing - can you post a short example Domain class? or perhaps update the test project (linked above) with the fix?

[dalelotts]

Hi Steve, et al;

This just keeps getting more interesting. The sample project is using standard generated controllers from Grails 2.3.7 - there is no call to .validate() prior to calling .hasErrros()

Adding
'''
customerTagTypeInstance.tenantId = 1;
'''
to the customer tag controller before the call to .hasErrors() does NOT fix the validation problem for me. (i.e. .hasErrors() still returns false)

As the code is written, it calls .hasErrrors() before calling .save() - no matter what I do, .hasErrors() returns false (There is one exception, setting tenantId to 1 in the form - for some reason .hasErrors() behaves correctly)

If I first call .validate() - it returns false and hasErrors() will then return true as expected. The documentation for .hasErrors() says it can report errors following data binding, since this is following data binding I would expect .hasErrors() to work without a call do validate(). I guess the authors of the controller templates thought so also since they did not include a call to validate()

I have no idea how a custom validator might correct this behavior, but I hope it will.

Ah, the 50 shades of Grails!

[lucastex]

Hi @dalelotts , just cloned your repo.
Can you give me some steps to reproduce the error?

[dalelotts]

Hi @lucastex - Hmm, I thought I included the steps in the readme file...anyway, here they are.

Steps to reproduce:
1 Run this project
2 Click on the "CustomerTagController"
3 create a new Customer Tag - perhaps "VIP" or whatever, and save it.
4 Now, create another customer tag with exactly the same name, and TRY to save it. I get a 500 error.

Repeat the steps above with the "TagController" - the validation error is correctly reported.

[lucastex]

Hi @dalelotts

Check this commit: lucastex/MultiTenantTest@eb59f5a

This is what we're saying on using custom validators.
It will not rely on database constraint. This is how we're using the MT Plugin in our app here (>100 MT Classes).

[lucastex]

Does it worked @dalelotts ?

[sronderos]

Hey Dale,

I think a previous version of this plugin took that approach, if I remember correctly there were some issues with that solution because other plugins may also try to hijack those methods. I'll see if I can figure out if that is a viable solution or if there is another more appropriate solution.

It looks like Lucas jumped in with another potential workaround for now. We need to look a little deeper into this plugin anyway with Hibernate 4 (which has a multi-tenant feature) now as a GORM implementation it may be worthwhile to upgrade this plugin to use those features.

-Steve

P.S. Sorry for closing and reopening the issue, hit the wrong button :-/

[dalelotts]

@lucastex The unique constraint should be on name and tenantId. For example, it is valid for two tenants to have a tag with the same name, but it is not valid for one tenant to have two tags with the same name. It looks to me like the constraint in your example is just a unique name constraint.

Furthermore, I don't think changing the custom validator to look for tenantId:name duplicates would solve the problem since the root of the issue is that tentantId is not yet set at the point at which the controller is performing the validation.

I admit I did not test this. If you tested the change and you see the correct behavior (i.e. tenantId:name unique constraint) please let me know.

[lucastex]

@dalelotts That was just an example of a custom validator trying to do it.
You can query for tenantId either, no problem at all, just create a criteria and analyse all your restrictions. 👍 if you want, I can commit another example using tenantId column either. What do you think?

@sronderos i haven't looked deeper into Hibernate4 MT feature, but AFAIK (from people that looked into it), the MT feature only works for different databases, or even different schemas in the same database.

[lucastex]

@sronderos yeah, this is right... The Hibernate 4 Multi Tenancy strategy for a "single-db" approach would be the DISCRIMINATOR strategy. Check its definition:

All data is kept in a single database schema. The data for each tenant is partitioned by 
the use of partition value or discriminator. The complexity of this discriminator might 
range from a simple column value to a complex SQL formula. Again, this approach would 
use a single Connection pool to service all tenants. However, in this approach the 
application needs to alter each and every SQL statement sent to the database to 
reference the “tenant identifier” discriminator.

Perfect! But:

This strategy is not yet implemented in Hibernate as of 4.0 and 4.1. Its support is planned for 5.0.

Sad but true :/