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

Schema-base multi-tenancy example #18

Closed
wants to merge 1 commit into from

Conversation

col-panic
Copy link

Concerning the Support for Multitenancy google-group discussion I implemented a concise example - this PR.

I would very much appreciate any comments on the code done here. It is not a prior target to get this PR included - but to have an appropriate discussion point for persons interested in this functionality.

@jamesagnew
Copy link
Contributor

This is really neat! Nice work.

I guess my main comment would be that it would be preferable to replace the tenant identification Filter with an implementation of ITenantIdentificationStrategy. This has a couple of advantages:

  • It is the HAPI FHIR native way of doing multitenancy, which means it'll play nicely with AuthorizationInterceptor and potentially other interceptors that are aware of it
  • It can be registered with the server at runtime, meaning that it could be enabled purely based on properties in the hapi.properties file

One other thought- it'd probably be good for this to have a unit test since it seems like something that will fail into disrepair pretty fast if it's not tested regularly. That's tough though of course, since that one has to work on Derby and I don't think Derby supports the USE clause you're using with MySQL.

Presumably we'd have to use a separate schema (i.e. a different JDBC URL) per tenant on Derby..

@col-panic
Copy link
Author

Thanks for your comment!

I don't see how FHIR itself, resp. ITenantIdentificationStrategy would fit to this scenario, if the "tenant-switch" is already done on JDBC resp. connection level. HAPI-FHIR shouldn't even bother here, no? I mean every request just operates against a totally different scheme.

I see that this patch is only working for mysql by now, for derby you would have to have a separate connection per tenant, which you then hand out. I'll take a look whether I can augment the starter with this scenario.

If HAPI-FHIR was to do the tenant handling, wouldn't it be only able to do single-database-multitenancy by means of tenant discriminators such that the SQL requests may consider them? ITenantIdentificationStrategy currently does not offer me any possibility to replace the underlying JDBC connection - but only to change the servers access url.

Unit tests - certainly - as soon as all discussion is done ;)

@jkiddo
Copy link
Collaborator

jkiddo commented May 22, 2021

@col-panic / @jamesagnew is this still relevant?

@col-panic
Copy link
Author

@jkiddo this was a prototype for a project that was not done. So for me it is not relevant, maybe it would be interesting to keep it if another developer runs into this?

@tsun424
Copy link

tsun424 commented Aug 24, 2021

Hi @col-panic , thanks for creating this PR and keeping it open for other developers. I wonder if you've ever implemented this schema-based multi-tenancy solution in any production environment, if you did, have you had any issues for this solution? Thank you.

@col-panic
Copy link
Author

@tsun424 I never really implemented this solution. Management decided to change direction ;)

@tsun424
Copy link

tsun424 commented Aug 27, 2021

Hi @col-panic thanks very much for your reply 👍

@tsun424 I never really implemented this solution. Management decided to change direction ;)

@patrick-werner
Copy link
Member

as this was only a prototype, im closing this PR

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.

None yet

5 participants