-
Notifications
You must be signed in to change notification settings - Fork 350
Allowing the override of duplicate connection exception #172
base: master
Are you sure you want to change the base?
Allowing the override of duplicate connection exception #172
Conversation
I would like somehow to be able to detect the duplicate connection exception on the server, which will allow me to implement a merge use case in my own code. The merge use case occurs if a member has registered an account with a social media provider (provider A), and then registered a second account with a different social media provider (provider B), and then tries to connect the account registered using provider B to provider A directly (when provider A has already been used to create an account). In more specific technical terms, when a duplicate connection exception occurs I want to override ConnectController to enable me to send an email along the lines of the following: "So you'd like to connect your account on website x to your google account? To do this, it seems it would be best to detect the duplicate connection exception and send the email at this point. The trouble is, this mechanism is currently private to the ConnectController so it cannot be overriden. Can we expose it please? |
I would think this is possible with the APIs already existing, just that you would have to check using thinks like provideruserid and your application’s userid to check for new duplicates that are associated with one or more of your application users. Spring Social has no idea what/where or anything about your own internal application user accounts to check. Only your own application would know that. I could also be mis-interpreting what you as looking for, and I am no expert, just my opinion here. Mark Spritzler
|
It may be true that there are alternative approaches and I have certainly experimented, but this seems the most practical method. The server is required to not only detect the duplicate connection exception / event but then return a sensible response to the UI after whatever non framework processing (sending an email for example) has occured. Controllers, in this case an overridden ConnectController method seems an excellent place to return an http response. I'd need more specifics in terms of implementation to consider an alternative approach that does not use ConnectController. I've clarified my comment based on your feedback! :) |
My question is how do you propose Spring Social to know about your User type in your application. It is different for everyone, and not even related to Facebook or Twitter or anything that Spring Social works with. It is like asking the Grocery store to come in and cook your dinners. The grocery store can sell you stuff to make your dinner, but it doesn’t know what you want for dinner or how to make it for you. That is my only point. Thanks Mark
|
I already use my own implementation of Spring Social's connection framework. http://docs.spring.io/spring-social/docs/1.1.x/reference/htmlsingle/#connectFramework I had no choice but to build my own implementation as I am using a graph database. This is the interface between my user accounts and spring social. For most people I expect they would use the out of the box implementation. Either way, this is not the point of issue. Using any implementation it should be possible to detect when there is a duplicate connection attempted server side. This can be done by overriding ConnectController. I'm all ears for an improved solution, but if there is not one, it would great if something similar to this could be pulled into the next version so I don't have to continue to fork the code. It may be useful for others also. For the sake of clarity, an example of how I use this change is below.
|
I see where this PR is going, but the more I think about it, I'm not a fan of things that require ConnectController to be extended. Ideally, ConnectController would be final (I'll likely never change it to be that way, but it should be treated that way as much as possible). My thinking is that the more use cases that require ConnectController to be overridden, the more difficult it is to auto-configure it in a Spring Boot app. For things like this, I think I'd rather have a strategy that defines the handling of the duplicate connection. Instead of calling handleDuplicateConnectionException(), invoke a similarly purposed method from an injected DuplicateConnectionHandlingStrategy. WDYT? |
Yes, I think something along those lines would be better although a strategy for only one exception seems in some way like overkill and a bit piecemeal? I notice DuplicateConnectionException is not the only exception being swallowed by ConnectController and also there are more reasons other than exception handling for extending ConnectController. Some kind of larger ConnectControllerStrategy that provides methods to optionally configure a range of ConnectController behaviour might be nice? What is your view on the behaviours that should be configurable by such a strategy? There's DuplicateConnectionException, but also Provider Error type exceptions. Maybe even connect and connectedView configuration? I may be able to have a crack at another pull request if you're interested. |
One more thought... I also seem to recall some annoyances at being able to only register a Connect / Disconnect Interceptor per Provider, but I could never see a way to register a general Interceptor for all providers. Ideally I'd be able to generally register for Connect and Disconnect events, and then make my own decisions about what I wanted to do with them based on their provider type :) This brings me to the thought that we might be able to create a more generalised version of Connect / Disconnect Interceptors, that fire on all Connect / Disconnect events, provide details such as Connection and Exception information (i.e. DuplicateConnectionException and / or others), and allow any relevant behaviour to be overridden if necessary? |
Closing PR and then will immediately reopen, in hopes of triggering Travis CI. |
Trying once more...Closing PR and then will immediately reopen, in hopes of triggering Travis CI. |
@johndeverall Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
No description provided.