Tenant Argument Binder #343
Conversation
|
|
||
| @RequestFilter | ||
| void filter(HttpRequest<?> request) { | ||
| Optional<Serializable> tenantOptional = resolveTenant(request); |
There was a problem hiding this comment.
I attempt first to resolve with a HttpRequestTenantResolver to avoid using ServerRequestContext if possible.
This adds an api `Tenant`, which represents the abstract notion of a tenant. Moreover, it adds a RequestFilter to resolve the tenant and set it as a request attribute. This pull request adds an argument binder that uses the request attribute resolved in the filter. Because of this, a user can bind the tenant as a controller method parameter
sdelamo
force-pushed
the
tenant-argument-binding
branch
from
7140d2d to
ceabab7
Compare
multitenancy/src/main/java/io/micronaut/multitenancy/filter/TenantResolverFilter.java
Show resolved
Hide resolved
...tenancy/src/main/java/io/micronaut/multitenancy/filter/TenantTypedRequestArgumentBinder.java
Outdated
Show resolved
Hide resolved
multitenancy/src/main/java/io/micronaut/multitenancy/Tenant.java
Outdated
Show resolved
Hide resolved
multitenancy/src/main/java/io/micronaut/multitenancy/filter/package-info.java
Outdated
Show resolved
Hide resolved
multitenancy/src/main/java/io/micronaut/multitenancy/filter/TenantResolverExistsCondition.java
Outdated
Show resolved
Hide resolved
multitenancy/src/main/java/io/micronaut/multitenancy/SerializableTenant.java
Outdated
Show resolved
Hide resolved
...tenancy/src/main/java/io/micronaut/multitenancy/filter/TenantTypedRequestArgumentBinder.java
Outdated
Show resolved
Hide resolved
.../main/java/io/micronaut/multitenancy/filter/TenantResolverFilterConfigurationProperties.java
Outdated
Show resolved
Hide resolved
...tenancy/src/main/java/io/micronaut/multitenancy/filter/TenantTypedRequestArgumentBinder.java
Outdated
Show resolved
Hide resolved
| * @param <T> The type of the tenant identifier | ||
| */ | ||
|
|
||
| public interface Tenant<T extends Serializable> { |
There was a problem hiding this comment.
Oh this is why.
I don't think this should be bound to Serializable, nobody uses java serialization.
There was a problem hiding this comment.
My original design return type for the id method was String.
ceabab7
@graemerocher thinks we should use a generic. And since tenant resolver returns serialisable, I set T extends Serializable
Maybe it makes sense to get back to return a String. We control how we set the attribute in the filter and we can call TenatnResolver::resolveIdentifier().toString. thoughts @graemerocher @yawkat ?
There was a problem hiding this comment.
Not even Hibernate requires a serializable ID
There was a problem hiding this comment.
Please don't use Serializable
There was a problem hiding this comment.
the choice was because TenatnResolver::resolveIdentifier uses serialisable and changing that now would be a breaking change.
There was a problem hiding this comment.
I added a String resolveId method to both TenantResolver and HttpTenantResolver. I have deprecated the Serializable resolveIdentifier methods. I use the resolveId in the filter now. Tenant is now simply :
public interface Tenant {
String id();
}
…nantResolverFilterConfigurationProperties.java Co-authored-by: Jonas Konrad <me@yawk.at>
| value = "${" + TenantResolverFilterConfigurationProperties.PREFIX + ".regex-pattern:" + TenantResolverFilterConfigurationProperties.DEFAULT_REGEX_PATTERN + "}") | ||
| @Internal | ||
| @Requires(condition = TenantResolverExistsCondition.class) | ||
| final class TenantResolverFilter implements Ordered { |
There was a problem hiding this comment.
Why do you add HTTP stuff in multinenancy? This should be in some HTTP module
There was a problem hiding this comment.
micronaut-multitenancy already depended on http-server. Most tenant resolvers were already http related. E.g.
CookieTenantResolver, HttpHeaderTenantResolver, `SubdomainTenantResolver
There was a problem hiding this comment.
micronaut-http-server is compileOnly and I have added a test which verifies you can load the module without http
| @@ -0,0 +1,6 @@ | |||
| The module ships a filter which resolves the current tenant and sets it as a request attribute named `tenantIdentifier`. | |||
There was a problem hiding this comment.
It would be better to have some TenantHttpAttributes.ID
There was a problem hiding this comment.
Not sure I understand what you are requesting. There is an already a constant with value tenantIdentifier and name TenantResolverFilter.ATTRIBUTE_TENANT.
|
This adds an api
Tenant, which represents the abstract notion of a tenant.Moreover, it adds a RequestFilter to resolve the tenant and set it as a request attribute.
This pull request adds an argument binder that uses the request attribute resolved in the filter. Because of this, a user can bind the tenant as a controller method parameter