updated multi-tenancy example to add TenantRepository and corrected t… #7643
Conversation
|
@mylen Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
|
@mylen Thank you for signing the Contributor License Agreement! |
spring-projects-issues
added
the
status: waiting-for-triage
There was a problem hiding this comment.
Thank you for the documentation improvements, @alexandre-melard! I've left some feedback inline.
| @@ -1273,13 +1296,13 @@ public class TenantAuthenticationManagerResolver implements AuthenticationManage | |||
|
|
|||
| @Override | |||
| public AuthenticationManager resolve(HttpServletRequest request) { | |||
| return this.authenticationManagers.computeIfAbsent(toTenant(request), this::fromTenant); <3> | |||
There was a problem hiding this comment.
Why was this removed? It should be linked to the <3> in the comments below.
There was a problem hiding this comment.
I see... The reference was aimed at the two lines. I thought it was just aimed at the second code referenced. My bad!
| [source,java] | ||
| ---- | ||
| @Component | ||
| public class TenantRepository { |
There was a problem hiding this comment.
I see your point in adding this sample TenantRepository. But, I think this demonstrates actually that it would be better to change the classes below to simply use a pair of Maps.
Would you mind changing the examples to use a Map<String, String> instead? Then, the members might be called issuersByTenant and jwkUrisByTenant instead of tenants or tenantRepository.
| @@ -1258,6 +1258,29 @@ http | |||
| Resolving the tenant by claim is similar to doing so by request material. | |||
| The only real difference is the `toTenant` method implementation: | |||
|
|
|||
| [source,java] | |||
There was a problem hiding this comment.
I think this snippet might be in the wrong place. Note the previous sentence, which is introducing the code snippet that is now below this one. I think we'd need to change the description a bit, too, for the reader to be able to follow.
jzheaux
added
in: docs
|
@alexandre-melard Are you able to apply the requested changes? |
|
Hi,
I have been too busy this week, I'll look into your comments early next
week.
Cheers
Alex
Le ven. 22 nov. 2019 à 00:43, Josh Cummings <notifications@github.com> a
écrit :
… @alexandre-melard <https://github.com/alexandre-melard> Are you able to
apply the requested changes?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7643?email_source=notifications&email_token=AAUHN7SS3K272GU7ZZHAFMLQU4MLHA5CNFSM4JMDQQH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE4BE5A#issuecomment-557322868>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAUHN7SJESBTHWDL4O2CQK3QU4MLHANCNFSM4JMDQQHQ>
.
|
spring-projects-issues
added
status: feedback-provided
jzheaux
added
status: waiting-for-feedback
|
This is actually obsolete now since #7733 was merged. However, @alexandre-melard, feel free to take a look at the existing docs and see if you'd like to make any more improvements! |
Corrected some typos in the documentation and added an example TenantRepository