Bug 357474

Comment 1 Doug Clarke 2011-09-22 08:04:23 EDT

I believe as we see more customers build SaaS with shared databases we'll see a demand for including the tenant identifier in the PK of their tables. This will allow customers to have unique natural/sequence values per tenant.

The implication of this is:
- INSERT must include the tenant identifier as with the non-PK case
- SELECTs must include the tenant identifier as with the non-PK case
- UPDATE must include all PK column values including the tenant identifier
- DELETE must include all PK column values including the tenant identifier
- Caching in the shared cache (not done my default with @Multitenant) only permitted if tenant identifier mapped.

If for 2.3.1 we require customers wanting the tenant identifier to be in the PK to map it as read-only where only EclipseLInk populates it we should cover the cases listed above. We could then log an enhancement request for our next release to fix up the cases where the tenant identifier is not mapped.

I am also comfortable with listing this configuration as a known issue for 2.3.1

Comment 2 Guy Pelletier 2011-09-26 16:01:42 EDT

Deferring this bug to 2.4 as 2.3.1 is closing down and there is not enough time to properly and fully address this bug.

We should document the following for 2.3.1 surrounding this feature:

1 - they can map the field
2 - they can avoid the L2 cache (default now with fix for bug 357476)
3 - current multitenant implementation expects uniqueness amongst id's

Comment 3 Guy Pelletier 2011-10-19 10:29:14 EDT

Created attachment 205533 [details]
Work in progress patch

I'm uploading this patch to outline my progress on this bug. By no means is the patch complete, rather a work in progress and thought it would be good if those who have an interest in this work could have a look at it and discuss if it aligns with their thoughts on the matter.

Quick low down of what is happening though. For any primary key tenant discriminator column, we add the column to the descriptors primary key list and also a new multitenant primary key mapping with a new multitenant accessor that is responsible for returning values from the mapping.

You'll notice a lot of the changes to files are minimal and needed only since we need to pass the session through now when extracting the value from a mapping (and used only by this new multitenant primary key mapping/accessor)

Metadata processing wise, primary key tenant discriminator columns are automatically added to relationship join columns and primary key join columns. We can sync up and re-use tenant discriminator columns across multitenant entities by using their associated context property.

Example:

@Entity
@Table(name="JPA_MAFIA_FAMILY")
@SecondaryTable(name="JPA_FAMILY_REVENUE")
// metadata processing will default:
//  @PrimaryKeyJoinColumns({
//    @PrimaryKeyJoinColumn(name="ID", referencedColumnName="ID")
//    @PrimaryKeyJoinColumn(name="TENANT_ID", referencedColumnName="TENANT_ID")
//  })
@Multitenant
@TenantDiscriminatorColumn(name="TENANT_ID", contextProperty="tenant.id", primaryKey = true)
public class MafiaFamily implements Serializable {
    ...
     
    @ElementCollection
    @CollectionTable(
      name="JPA_FAMILY_TAGS",
      joinColumns={
          @JoinColumn(name="FAMILY_ID", referencedColumnName="ID")
	  // metadata processing will add:
	  // @JoinColumn(name="TENANT_ID", referencedColumn="TENANT_ID")
      })
    @Column(name="TAG")
    public Collection<String> getTags() { 
        return tags; 
    }

    @Basic
    @Column(name="REVENUE", table="JPA_FAMILY_REVENUE")
    public Double getRevenue() { 
        return revenue; 
    }

    ...
}

@Entity
@Table(name="JPA_MAFIOSO")
@Multitenant
@TenantDiscriminatorColumn(name="T_ID", contextProperty="tenant.id")
@Inheritance(strategy=JOINED)
@DiscriminatorColumn(name="DTYPE")
public abstract class Mafioso {
    ...
    
    @ManyToOne
    @JoinColumns({
        @JoinColumn(name="FAMILY_ID", referencedColumnName="ID")
	// metadata processing will add:
	// @JoinColumn(name="T_ID", referenceColumnName="TENANT_ID", updatable=false, insertable=false)
	// if Mafioso was not @Multitenant, metadata processing would add:
 	// @JoinColumn(name="TENANT_ID", referenceColumnName="TENANT_ID")
    })
    public MafiaFamily getFamily() { 
        return family; 
    }
    
    ...
}

The whole idea of adding/syncing primary key tenant discriminator columns gets a little more complex when multiples are used. Also begs to ask how many people will want/use multiples ... regardless, it should be possible to address either way.

When a primary key tenant discriminator column is specified, any tables (collection, join-table, secondary-table etc.) related to the entity must have an equivalent column specified. 

Current DDL generation solution will add the tenant discriminator primary key columns to all tables, from example above:

CREATE TABLE JPA_MAFIA_FAMILY (TENANT_ID VARCHAR(31) NOT NULL, ID INTEGER NOT NULL, NAME VARCHAR(255), PRIMARY KEY (TENANT_ID, ID))
CREATE TABLE JPA_FAMILY_REVENUE (ID INTEGER NOT NULL, TENANT_ID VARCHAR(31) NOT NULL, REVENUE DOUBLE, PRIMARY KEY (ID, TENANT_ID))
CREATE TABLE JPA_FAMILY_TAGS (FAMILY_ID INTEGER, TENANT_ID VARCHAR(31), TAG VARCHAR(255))

ALTER TABLE JPA_FAMILY_REVENUE ADD CONSTRAINT FK_JPA_FAMILY_REVENUE_TENANT_ID FOREIGN KEY (TENANT_ID, ID) REFERENCES JPA_MAFIA_FAMILY (TENANT_ID, ID)
ALTER TABLE JPA_FAMILY_TAGS ADD CONSTRAINT FK_JPA_FAMILY_TAGS_TENANT_ID FOREIGN KEY (TENANT_ID, FAMILY_ID) REFERENCES JPA_MAFIA_FAMILY (TENANT_ID, ID)

Comment 4 Guy Pelletier 2011-10-20 10:51:56 EDT

Created attachment 205636 [details]
refactor patch (wip)

Comment 5 Guy Pelletier 2011-10-20 10:52:29 EDT

Created attachment 205637 [details]
feature patch (wip)

Comment 6 Guy Pelletier 2011-11-01 13:17:50 EDT

Created attachment 206278 [details]
Proposed changes

Refactored AbstractDirectMapping to have two new subclasses (AbstractAttributeDirectMapping and AbstractPropertyDirectMapping).

New MultitenantPrimaryKeyMapping subclasses AbstractPropertyDirectMapping. The thinking that it is a direct mapping minus the attribute portion (from DatabaseMapping).

DirectToFieldMapping (and subclasses) inherit from AbstractAttributeDirectMapping. 

Some methods that apply to 'attribute' mappings have been left in AbstractDirectMapping but deprecated. AbstractAttributeDirectMapping provides overridden API to alleviate the deprecated warnings.

Comment 7 Guy Pelletier 2011-11-09 10:27:19 EST

Created attachment 206702 [details]
Proposed changes

Comment 8 Guy Pelletier 2011-11-11 09:50:29 EST

Patch (with a couple minor commenting changes) has been checked in.

Verified by: Gordon Yorke, James Sutherland

Transaction has been verified and re-worked a number of times. This is a stamp check in. Some reviews are ongoing therefore an update may need to be provided, however I expect any updates to be minor at this point.

Leaving the transaction open for the time being.

Tests: Existing multitenant model tweaked to make use of the primary key feature. New test (testMultitenantPrimaryKeyWithIdClass) added to AdvancedMultitenantJunitTest suite. Other tests from that suite continue to pass. Core SRG and LRG passed successfully.

Comment 9 Guy Pelletier 2011-11-16 09:56:52 EST

Created attachment 207089 [details]
Update patch 1

Some abstracting and deprecating from AbstractDirectMapping based on feedback from Gordon Yorke.

Comment 10 Guy Pelletier 2011-11-24 09:31:17 EST

Created attachment 207482 [details]
Updates

Comment 11 Guy Pelletier 2011-11-25 11:06:19 EST

Created attachment 207540 [details]
Updates

Comment 12 Guy Pelletier 2011-11-25 13:28:28 EST

Created attachment 207552 [details]
Updates

Comment 13 Guy Pelletier 2011-11-25 13:56:22 EST

Updates have been submitted.

Verified by: Gordon Yorke

Tests: Existing multitenant model tests continue to pass. Core SRG and LRG passed successfully.

Comment 14 Karen Butzke 2011-12-20 09:40:54 EST

Guy,
Is this a valid use case? I have no primary key defined except for the tenant discriminator column. I get no EclipseLink exceptions during login.

@Entity
@Multitenant(value=MultitenantType.SINGLE_TABLE)
@TenantDiscriminatorColumn(name="ID", primaryKey=true)
public class Foo {


}

Comment 15 Eclipse Webmaster 2022-06-09 10:24:54 EDT

The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink