On 30/01/2013 13:25, Christian Schneider wrote:
Hi Francesco,

I am currently working on the patch to move the
InvalidSearchConditionException and make it checked again.
I am not sure if making it checked is a good idea. As the exception will
now be part of the service interface I will have to add it to lots of
methods. None of these methods really handles this exception.
The reason for this is that before we used the RestTemplate to access
the services. So it did not matter if the service throws a checked
exception. We would not be forced to catch it. Now with the interfaces
for the services we have to add the checked exception to the service
interface if we want the impl to be able to throw it. So the client is
also affected by that.

Clear.

In the end the exception now also appears in AttributableDataProvider
which can not rethrow the exception as it would violate the
IDataProvider interface. So I would have to catch it there but I have no
idea what to do if it happens.

As said before, this should never happen on the admin console since the wrapper is architected to generate only valid conditions and to check for correctness; anyway, the catch() block should only be able to log and report this error to the user.

I created a gist of the patch so you can have a look:
https://gist.github.com/4672932

So I see two possible solutions to improve this:
1) Make the exception unchecked again so it can be thrown by the
services but has no effect on the client side
2) Catch the exception in the server impls and rethrow it as a
RuntimeException so it does not affect the clients

If the admin console is the only problem, we can easily find a fix there.

If you still think having this checked is too much problematic for you, just leave it unchecked.

Regards.

On 30.01.2013 08:29, Francesco Chicchiriccò wrote:
On 28/01/2013 15:38, Francesco Chicchiriccò wrote:
On 28/01/2013 15:36, Christian Schneider wrote:
Currently InvalidSearchConditionException is in core.persistence.dao
and
is a checked exception.
If we want to make this exception part of the UserService interface to
have it available on the client we will have to move it to common.

Currently the console does not seem to use the exception on the client
side. So my preliminary solution to get it running is to make the
exception unchecked so I can pass it to the
exception mapper without having it in the interface.

So basically the two things to discuss are if we want to move the
exception to common and if we want to make it unchecked. I personally
would do both.
This exception is not used by the admin console because the code is
done so that no invalid search conditions are sent to the core;
however, other clients are likely to need this exception to be returned.

Hence: +1 for moving it to common, but don't see enough reason to
make it unchecked.
I've noticed that this issue was moved to common as unchecked: any
additional reason behind not leaving it checked than the need to
change some interface method's signature?

Regards.

--
Francesco Chicchiriccò

ASF Member, Apache Syncope PMC chair, Apache Cocoon PMC Member
http://people.apache.org/~ilgrosso/

Reply via email to