subject:"Re\: adaptTo and results ...."

Regarding 1) Having such a Result class would mean that all consumer would need 
to unwrap the exception first. So instead of being forced of implementing a 
null-check (as with the old solution) one would need to implement another 
check. I want to prevent such a burden to the consumers.
Regarding 2) Since the client code knows on which object the adaptToOrThrow is 
called I don’t get why an instanceof check is necessary. Whether this object 
implements Adaptable2 is known at compile-time, so you do have the full 
IDE-support here.
Regarding 3) In that case I would no longer allow a null value to be returned. 
One drawback is, that all the null checks are no longer effective.

IMHO solution 2) is the best. At the same time I would deprecate the old 
Adaptable, because I cannot come up with a real use-case where returning has an 
advantage. I would rather implement another method boolean 
hasAdapter(ClassAdapterType type) on the Adaptable2 interface.
Regards,
Konrad

On 01 Jul 2014, at 09:07, Felix Meschberger fmesc...@adobe.com wrote:

 Hi
 
 There currently are two issues floating around dealing with the question of 
 returning more information than just null from the Adaptable.adaptTo(Class) 
 method: https://issues.apache.org/jira/browse/SLING-3714 and 
 https://issues.apache.org/jira/browse/SLING-3709. I think these requests 
 warrant some discussion on the list.
 
 Background: adaptTo can be implemented by Adaptable implementations 
 themselves or, by extending from SlingAdaptable, they may defer to an 
 AdapterMananager. AdapterManager itself is extended by AdapterFactory 
 services. All these interfaces define an adaptTo method. All these methods 
 return null if adaption is not possible and don't declare or document to 
 throw an exception.
 
 While not explicitly documented as such, the intention is and was that 
 adaptTo never throws on the grounds that adaption may fail which is 
 considered a valid result and thus exceptions are not to be expected and 
 handled.
 
 Hence all implementations of the methods generally 
 catch-and-log-but-don't-throw. Interestingly SlingAdaptable.adaptTo and 
 AdapterManagerImpl.getAdapter don't catch — so any RuntimeException thrown 
 from an AdapterFactory would be forwarded.
 
 Having said this there are options available:
 
 (1) Add support for a new Result? class. We would probably implement this 
 in the AdapterManager.getAdapter implementation explicitly handling this case 
 because it would entail catching the adaptTo/getAdapter calls to get the 
 exception (the Result.getError should return Throwable probably not Error)
 
 Use would be limited to new AdapterFactory implementations throwing 
 RuntimeExcpetion. For Sling Models this would be the case.
 
 (2) Add a new adaptToOrThrow method, which is declared to throw a 
 RuntimeException and never return null: Either it can adapt or it throws. 
 This would require a new interface Adaptable2 (probably) to not break 
 existing Adaptable implementations. The SlingAdaptable base class would 
 implement the new method of course, probably something like this:
 
 SlingAdaptable implements Adaptable2 {
  …
  public AdapterType AdapterType adaptToOrThrow(ClassAdapterType type) {
  AdapterType result = this.adaptTo(type);
  if (result != null) {
  return result;
  }
  throw new CannotAdaptException(…);
  }
 }
 
 
 Use is problematic because you would have to know whether you can call the 
 new method: So instead of an null check you now have an instanceof check … 
 Except for the Resource interface which would be extended to extend from 
 Adaptable2 as well.
 
 (3) Document, that Adaptable.adaptTo may throw a RuntimeException.
 
 The problem here is, that this may conceptually break existing callers of 
 Adaptable.adaptTo which don't expect an exception at all — presumably this is 
 a minor nuisance because technically a RuntimeException may always be thrown.
 
 Regards
 Felix

Re: adaptTo and results ....

Adding a new interface would require us to implement it all over the place
and as Felix points out, client code would always need to check whether the
new interface is implemented or not Having to methods, like hasAdapter and
adaptOrThrow does not work very well as between the two calls things might
have changed already: while hasAdapter returns true, the underlying factory
gets unregistered before adaptOrThrow is called.
In many use cases, the current pattern works fine - the client does not
care whether an exception is thrown within the adaption - it just cares
whether an object is returned or not. And there are valid use cases, where
client code does different things whether the adaption works or not (e.g.
the post servlet checks for adaptTo(Node) and then does additional things
if the resource is backed up by a node.)

I see the point that there are also use cases where it would be fine to
simpy throw an exception if adaptTo is not successful. This would make the
client code easier. However as this most properly is a runtime exception,
client code can today just call a method on the object and end up with a
NPE - having the same result :)

This leaves us with use cases where the client code explicitely wants to
catch the exception and then do something depending on the exception. Maybe
we should just add something for this explicit use case instead of bloating
the general adaptTo mechanism?

Regards
Carsten




2014-07-01 9:44 GMT+02:00 Konrad Windszus konra...@gmx.de:

 Regarding 1) Having such a Result class would mean that all consumer would
 need to unwrap the exception first. So instead of being forced of
 implementing a null-check (as with the old solution) one would need to
 implement another check. I want to prevent such a burden to the consumers.
 Regarding 2) Since the client code knows on which object the
 adaptToOrThrow is called I don’t get why an instanceof check is necessary.
 Whether this object implements Adaptable2 is known at compile-time, so you
 do have the full IDE-support here.
 Regarding 3) In that case I would no longer allow a null value to be
 returned. One drawback is, that all the null checks are no longer effective.

 IMHO solution 2) is the best. At the same time I would deprecate the old
 Adaptable, because I cannot come up with a real use-case where returning
 has an advantage. I would rather implement another method boolean
 hasAdapter(ClassAdapterType type) on the Adaptable2 interface.
 Regards,
 Konrad

 On 01 Jul 2014, at 09:07, Felix Meschberger fmesc...@adobe.com wrote:

  Hi
 
  There currently are two issues floating around dealing with the question
 of returning more information than just null from the
 Adaptable.adaptTo(Class) method:
 https://issues.apache.org/jira/browse/SLING-3714 and
 https://issues.apache.org/jira/browse/SLING-3709. I think these requests
 warrant some discussion on the list.
 
  Background: adaptTo can be implemented by Adaptable implementations
 themselves or, by extending from SlingAdaptable, they may defer to an
 AdapterMananager. AdapterManager itself is extended by AdapterFactory
 services. All these interfaces define an adaptTo method. All these methods
 return null if adaption is not possible and don't declare or document to
 throw an exception.
 
  While not explicitly documented as such, the intention is and was that
 adaptTo never throws on the grounds that adaption may fail which is
 considered a valid result and thus exceptions are not to be expected and
 handled.
 
  Hence all implementations of the methods generally
 catch-and-log-but-don't-throw. Interestingly SlingAdaptable.adaptTo and
 AdapterManagerImpl.getAdapter don't catch — so any RuntimeException thrown
 from an AdapterFactory would be forwarded.
 
  Having said this there are options available:
 
  (1) Add support for a new Result? class. We would probably implement
 this in the AdapterManager.getAdapter implementation explicitly handling
 this case because it would entail catching the adaptTo/getAdapter calls to
 get the exception (the Result.getError should return Throwable probably not
 Error)
 
  Use would be limited to new AdapterFactory implementations throwing
 RuntimeExcpetion. For Sling Models this would be the case.
 
  (2) Add a new adaptToOrThrow method, which is declared to throw a
 RuntimeException and never return null: Either it can adapt or it throws.
 This would require a new interface Adaptable2 (probably) to not break
 existing Adaptable implementations. The SlingAdaptable base class would
 implement the new method of course, probably something like this:
 
  SlingAdaptable implements Adaptable2 {
   …
   public AdapterType AdapterType adaptToOrThrow(ClassAdapterType
 type) {
   AdapterType result = this.adaptTo(type);
   if (result != null) {
   return result;
   }
   throw new CannotAdaptException(…);
   }
  }
 
 
  Use is problematic because you would have to know whether you can call
 the new method: So instead of

Re: adaptTo and results ....

adaptTo() is currently commonly used as a test, similar to instanceof.
Throwing and catching to return null is a very poor implementation
(performance-wise) for this use.

Adding a hasAdapter() or canAdaptTo() might decrease the number of
implementations that think throwing is OK, but only if the new interface
is visible at the level of the specific implementation.  (If they're just
done as wrappers with catches somewhere in the machinery, and the
AdapterFactory writer is unaware of them, then they probably don't help
this issue.)

Cheers,
Jeff.


On 01/07/2014 08:44, Konrad Windszus konra...@gmx.de wrote:

Regarding 1) Having such a Result class would mean that all consumer
would need to unwrap the exception first. So instead of being forced of
implementing a null-check (as with the old solution) one would need to
implement another check. I want to prevent such a burden to the consumers.
Regarding 2) Since the client code knows on which object the
adaptToOrThrow is called I don¹t get why an instanceof check is
necessary. Whether this object implements Adaptable2 is known at
compile-time, so you do have the full IDE-support here.
Regarding 3) In that case I would no longer allow a null value to be
returned. One drawback is, that all the null checks are no longer
effective.

IMHO solution 2) is the best. At the same time I would deprecate the old
Adaptable, because I cannot come up with a real use-case where returning
has an advantage. I would rather implement another method boolean
hasAdapter(ClassAdapterType type) on the Adaptable2 interface.
Regards,
Konrad

On 01 Jul 2014, at 09:07, Felix Meschberger fmesc...@adobe.com wrote:

 Hi
 
 There currently are two issues floating around dealing with the
question of returning more information than just null from the
Adaptable.adaptTo(Class) method:
https://issues.apache.org/jira/browse/SLING-3714 and
https://issues.apache.org/jira/browse/SLING-3709. I think these requests
warrant some discussion on the list.
 
 Background: adaptTo can be implemented by Adaptable implementations
themselves or, by extending from SlingAdaptable, they may defer to an
AdapterMananager. AdapterManager itself is extended by AdapterFactory
services. All these interfaces define an adaptTo method. All these
methods return null if adaption is not possible and don't declare or
document to throw an exception.
 
 While not explicitly documented as such, the intention is and was that
adaptTo never throws on the grounds that adaption may fail which is
considered a valid result and thus exceptions are not to be expected and
handled.
 
 Hence all implementations of the methods generally
catch-and-log-but-don't-throw. Interestingly SlingAdaptable.adaptTo and
AdapterManagerImpl.getAdapter don't catch ‹ so any RuntimeException
thrown from an AdapterFactory would be forwarded.
 
 Having said this there are options available:
 
 (1) Add support for a new Result? class. We would probably implement
this in the AdapterManager.getAdapter implementation explicitly handling
this case because it would entail catching the adaptTo/getAdapter calls
to get the exception (the Result.getError should return Throwable
probably not Error)
 
 Use would be limited to new AdapterFactory implementations throwing
RuntimeExcpetion. For Sling Models this would be the case.
 
 (2) Add a new adaptToOrThrow method, which is declared to throw a
RuntimeException and never return null: Either it can adapt or it
throws. This would require a new interface Adaptable2 (probably) to not
break existing Adaptable implementations. The SlingAdaptable base class
would implement the new method of course, probably something like this:
 
 SlingAdaptable implements Adaptable2 {
  Š
  public AdapterType AdapterType adaptToOrThrow(ClassAdapterType
type) {
  AdapterType result = this.adaptTo(type);
  if (result != null) {
  return result;
  }
  throw new CannotAdaptException(Š);
  }
 }
 
 
 Use is problematic because you would have to know whether you can call
the new method: So instead of an null check you now have an instanceof
check Š Except for the Resource interface which would be extended to
extend from Adaptable2 as well.
 
 (3) Document, that Adaptable.adaptTo may throw a RuntimeException.
 
 The problem here is, that this may conceptually break existing callers
of Adaptable.adaptTo which don't expect an exception at all ‹ presumably
this is a minor nuisance because technically a RuntimeException may
always be thrown.
 
 Regards
 Felix

Re: adaptTo and results ....

It is not (only) about throwing exceptions in case no suitable adapter is 
available. It rather is about the fact, that today the adaptTo is a barrier for 
all kinds of exceptions. In some cases the adaptation fails for a specific 
reason (one example is Sling Models where injection fails, another one is the 
issue mentioned in https://issues.apache.org/jira/browse/SLING-2712 (ValueMap 
not supporting primitives)). Both would be valid use cases where the client 
would be interested to catch the exception here.

On 01 Jul 2014, at 10:34, Carsten Ziegeler cziege...@apache.org wrote:

 Adding a new interface would require us to implement it all over the place
 and as Felix points out, client code would always need to check whether the
 new interface is implemented or not Having to methods, like hasAdapter and
 adaptOrThrow does not work very well as between the two calls things might
 have changed already: while hasAdapter returns true, the underlying factory
 gets unregistered before adaptOrThrow is called.
 In many use cases, the current pattern works fine - the client does not
 care whether an exception is thrown within the adaption - it just cares
 whether an object is returned or not. And there are valid use cases, where
 client code does different things whether the adaption works or not (e.g.
 the post servlet checks for adaptTo(Node) and then does additional things
 if the resource is backed up by a node.)
 
 I see the point that there are also use cases where it would be fine to
 simpy throw an exception if adaptTo is not successful. This would make the
 client code easier. However as this most properly is a runtime exception,
 client code can today just call a method on the object and end up with a
 NPE - having the same result :)
 
 This leaves us with use cases where the client code explicitely wants to
 catch the exception and then do something depending on the exception. Maybe
 we should just add something for this explicit use case instead of bloating
 the general adaptTo mechanism?
 
 Regards
 Carsten
 
 
 
 
 2014-07-01 9:44 GMT+02:00 Konrad Windszus konra...@gmx.de:
 
 Regarding 1) Having such a Result class would mean that all consumer would
 need to unwrap the exception first. So instead of being forced of
 implementing a null-check (as with the old solution) one would need to
 implement another check. I want to prevent such a burden to the consumers.
 Regarding 2) Since the client code knows on which object the
 adaptToOrThrow is called I don’t get why an instanceof check is necessary.
 Whether this object implements Adaptable2 is known at compile-time, so you
 do have the full IDE-support here.
 Regarding 3) In that case I would no longer allow a null value to be
 returned. One drawback is, that all the null checks are no longer effective.
 
 IMHO solution 2) is the best. At the same time I would deprecate the old
 Adaptable, because I cannot come up with a real use-case where returning
 has an advantage. I would rather implement another method boolean
 hasAdapter(ClassAdapterType type) on the Adaptable2 interface.
 Regards,
 Konrad
 
 On 01 Jul 2014, at 09:07, Felix Meschberger fmesc...@adobe.com wrote:
 
 Hi
 
 There currently are two issues floating around dealing with the question
 of returning more information than just null from the
 Adaptable.adaptTo(Class) method:
 https://issues.apache.org/jira/browse/SLING-3714 and
 https://issues.apache.org/jira/browse/SLING-3709. I think these requests
 warrant some discussion on the list.
 
 Background: adaptTo can be implemented by Adaptable implementations
 themselves or, by extending from SlingAdaptable, they may defer to an
 AdapterMananager. AdapterManager itself is extended by AdapterFactory
 services. All these interfaces define an adaptTo method. All these methods
 return null if adaption is not possible and don't declare or document to
 throw an exception.
 
 While not explicitly documented as such, the intention is and was that
 adaptTo never throws on the grounds that adaption may fail which is
 considered a valid result and thus exceptions are not to be expected and
 handled.
 
 Hence all implementations of the methods generally
 catch-and-log-but-don't-throw. Interestingly SlingAdaptable.adaptTo and
 AdapterManagerImpl.getAdapter don't catch — so any RuntimeException thrown
 from an AdapterFactory would be forwarded.
 
 Having said this there are options available:
 
 (1) Add support for a new Result? class. We would probably implement
 this in the AdapterManager.getAdapter implementation explicitly handling
 this case because it would entail catching the adaptTo/getAdapter calls to
 get the exception (the Result.getError should return Throwable probably not
 Error)
 
 Use would be limited to new AdapterFactory implementations throwing
 RuntimeExcpetion. For Sling Models this would be the case.
 
 (2) Add a new adaptToOrThrow method, which is declared to throw a
 RuntimeException and never return null: Either it can adapt or it

Re: adaptTo and results ....

adaption and validation are different concerns

Carsten


2014-07-01 10:55 GMT+02:00 Jeff Young j...@adobe.com:

 We could solve that by defining a specific exception for
 adaptation-not-possible and then catch only that.

 Of course that would leak tons of exceptions from code written before that
 exception became available.  Maybe do the catching based on some sort of
 version clue?

 Cheers,
 Jeff.


 On 01/07/2014 09:40, Konrad Windszus konra...@gmx.de wrote:

 It is not (only) about throwing exceptions in case no suitable adapter is
 available. It rather is about the fact, that today the adaptTo is a
 barrier for all kinds of exceptions. In some cases the adaptation fails
 for a specific reason (one example is Sling Models where injection fails,
 another one is the issue mentioned in
 https://issues.apache.org/jira/browse/SLING-2712 (ValueMap not supporting
 primitives)). Both would be valid use cases where the client would be
 interested to catch the exception here.
 
 On 01 Jul 2014, at 10:34, Carsten Ziegeler cziege...@apache.org wrote:
 
  Adding a new interface would require us to implement it all over the
 place
  and as Felix points out, client code would always need to check whether
 the
  new interface is implemented or not Having to methods, like hasAdapter
 and
  adaptOrThrow does not work very well as between the two calls things
 might
  have changed already: while hasAdapter returns true, the underlying
 factory
  gets unregistered before adaptOrThrow is called.
  In many use cases, the current pattern works fine - the client does not
  care whether an exception is thrown within the adaption - it just cares
  whether an object is returned or not. And there are valid use cases,
 where
  client code does different things whether the adaption works or not
 (e.g.
  the post servlet checks for adaptTo(Node) and then does additional
 things
  if the resource is backed up by a node.)
 
  I see the point that there are also use cases where it would be fine to
  simpy throw an exception if adaptTo is not successful. This would make
 the
  client code easier. However as this most properly is a runtime
 exception,
  client code can today just call a method on the object and end up with a
  NPE - having the same result :)
 
  This leaves us with use cases where the client code explicitely wants to
  catch the exception and then do something depending on the exception.
 Maybe
  we should just add something for this explicit use case instead of
 bloating
  the general adaptTo mechanism?
 
  Regards
  Carsten
 
 
 
 
  2014-07-01 9:44 GMT+02:00 Konrad Windszus konra...@gmx.de:
 
  Regarding 1) Having such a Result class would mean that all consumer
 would
  need to unwrap the exception first. So instead of being forced of
  implementing a null-check (as with the old solution) one would need to
  implement another check. I want to prevent such a burden to the
 consumers.
  Regarding 2) Since the client code knows on which object the
  adaptToOrThrow is called I don¹t get why an instanceof check is
 necessary.
  Whether this object implements Adaptable2 is known at compile-time, so
 you
  do have the full IDE-support here.
  Regarding 3) In that case I would no longer allow a null value to be
  returned. One drawback is, that all the null checks are no longer
 effective.
 
  IMHO solution 2) is the best. At the same time I would deprecate the
 old
  Adaptable, because I cannot come up with a real use-case where
 returning
  has an advantage. I would rather implement another method boolean
  hasAdapter(ClassAdapterType type) on the Adaptable2 interface.
  Regards,
  Konrad
 
  On 01 Jul 2014, at 09:07, Felix Meschberger fmesc...@adobe.com
 wrote:
 
  Hi
 
  There currently are two issues floating around dealing with the
 question
  of returning more information than just null from the
  Adaptable.adaptTo(Class) method:
  https://issues.apache.org/jira/browse/SLING-3714 and
  https://issues.apache.org/jira/browse/SLING-3709. I think these
 requests
  warrant some discussion on the list.
 
  Background: adaptTo can be implemented by Adaptable implementations
  themselves or, by extending from SlingAdaptable, they may defer to an
  AdapterMananager. AdapterManager itself is extended by AdapterFactory
  services. All these interfaces define an adaptTo method. All these
 methods
  return null if adaption is not possible and don't declare or document
 to
  throw an exception.
 
  While not explicitly documented as such, the intention is and was that
  adaptTo never throws on the grounds that adaption may fail which is
  considered a valid result and thus exceptions are not to be expected
 and
  handled.
 
  Hence all implementations of the methods generally
  catch-and-log-but-don't-throw. Interestingly SlingAdaptable.adaptTo and
  AdapterManagerImpl.getAdapter don't catch ‹ so any RuntimeException
 thrown
  from an AdapterFactory would be forwarded.
 
  Having said this there are options available:
 
  (1)

Re: adaptTo and results ....

Hi Carsten,

Can you say more?  (I'm not sure I understand what you're getting at)

Thanks,
Jeff.


On 01/07/2014 09:56, Carsten Ziegeler cziege...@apache.org wrote:

adaption and validation are different concerns

Carsten


2014-07-01 10:55 GMT+02:00 Jeff Young j...@adobe.com:

 We could solve that by defining a specific exception for
 adaptation-not-possible and then catch only that.

 Of course that would leak tons of exceptions from code written before
that
 exception became available.  Maybe do the catching based on some sort of
 version clue?

 Cheers,
 Jeff.


 On 01/07/2014 09:40, Konrad Windszus konra...@gmx.de wrote:

 It is not (only) about throwing exceptions in case no suitable adapter
is
 available. It rather is about the fact, that today the adaptTo is a
 barrier for all kinds of exceptions. In some cases the adaptation fails
 for a specific reason (one example is Sling Models where injection
fails,
 another one is the issue mentioned in
 https://issues.apache.org/jira/browse/SLING-2712 (ValueMap not
supporting
 primitives)). Both would be valid use cases where the client would be
 interested to catch the exception here.
 
 On 01 Jul 2014, at 10:34, Carsten Ziegeler cziege...@apache.org
wrote:
 
  Adding a new interface would require us to implement it all over the
 place
  and as Felix points out, client code would always need to check
whether
 the
  new interface is implemented or not Having to methods, like
hasAdapter
 and
  adaptOrThrow does not work very well as between the two calls things
 might
  have changed already: while hasAdapter returns true, the underlying
 factory
  gets unregistered before adaptOrThrow is called.
  In many use cases, the current pattern works fine - the client does
not
  care whether an exception is thrown within the adaption - it just
cares
  whether an object is returned or not. And there are valid use cases,
 where
  client code does different things whether the adaption works or not
 (e.g.
  the post servlet checks for adaptTo(Node) and then does additional
 things
  if the resource is backed up by a node.)
 
  I see the point that there are also use cases where it would be fine
to
  simpy throw an exception if adaptTo is not successful. This would
make
 the
  client code easier. However as this most properly is a runtime
 exception,
  client code can today just call a method on the object and end up
with a
  NPE - having the same result :)
 
  This leaves us with use cases where the client code explicitely
wants to
  catch the exception and then do something depending on the exception.
 Maybe
  we should just add something for this explicit use case instead of
 bloating
  the general adaptTo mechanism?
 
  Regards
  Carsten
 
 
 
 
  2014-07-01 9:44 GMT+02:00 Konrad Windszus konra...@gmx.de:
 
  Regarding 1) Having such a Result class would mean that all consumer
 would
  need to unwrap the exception first. So instead of being forced of
  implementing a null-check (as with the old solution) one would need
to
  implement another check. I want to prevent such a burden to the
 consumers.
  Regarding 2) Since the client code knows on which object the
  adaptToOrThrow is called I don¹t get why an instanceof check is
 necessary.
  Whether this object implements Adaptable2 is known at compile-time,
so
 you
  do have the full IDE-support here.
  Regarding 3) In that case I would no longer allow a null value to be
  returned. One drawback is, that all the null checks are no longer
 effective.
 
  IMHO solution 2) is the best. At the same time I would deprecate the
 old
  Adaptable, because I cannot come up with a real use-case where
 returning
  has an advantage. I would rather implement another method boolean
  hasAdapter(ClassAdapterType type) on the Adaptable2 interface.
  Regards,
  Konrad
 
  On 01 Jul 2014, at 09:07, Felix Meschberger fmesc...@adobe.com
 wrote:
 
  Hi
 
  There currently are two issues floating around dealing with the
 question
  of returning more information than just null from the
  Adaptable.adaptTo(Class) method:
  https://issues.apache.org/jira/browse/SLING-3714 and
  https://issues.apache.org/jira/browse/SLING-3709. I think these
 requests
  warrant some discussion on the list.
 
  Background: adaptTo can be implemented by Adaptable implementations
  themselves or, by extending from SlingAdaptable, they may defer to
an
  AdapterMananager. AdapterManager itself is extended by
AdapterFactory
  services. All these interfaces define an adaptTo method. All these
 methods
  return null if adaption is not possible and don't declare or
document
 to
  throw an exception.
 
  While not explicitly documented as such, the intention is and was
that
  adaptTo never throws on the grounds that adaption may fail which is
  considered a valid result and thus exceptions are not to be expected
 and
  handled.
 
  Hence all implementations of the methods generally
  catch-and-log-but-don't-throw. Interestingly SlingAdaptable.adaptTo
and

Re: adaptTo and results ....

Sure :) For the adapter pattern, the client does not care why the adaption
failed, the client is just interested in the result (success or not)
Validation is a different beast, if validation fails you want to know
specific reasons why it failed - and this can be multiple.
I tried to explain in my first mail on this thread, that all other use
cases mentioned can be handled with the current implementation - with the
exception of validation. But I think validation requires a different
concept than the adapter pattern.

Carsten


2014-07-01 11:09 GMT+02:00 Jeff Young j...@adobe.com:

 Hi Carsten,

 Can you say more?  (I'm not sure I understand what you're getting at)

 Thanks,
 Jeff.


 On 01/07/2014 09:56, Carsten Ziegeler cziege...@apache.org wrote:

 adaption and validation are different concerns
 
 Carsten
 
 
 2014-07-01 10:55 GMT+02:00 Jeff Young j...@adobe.com:
 
  We could solve that by defining a specific exception for
  adaptation-not-possible and then catch only that.
 
  Of course that would leak tons of exceptions from code written before
 that
  exception became available.  Maybe do the catching based on some sort of
  version clue?
 
  Cheers,
  Jeff.
 
 
  On 01/07/2014 09:40, Konrad Windszus konra...@gmx.de wrote:
 
  It is not (only) about throwing exceptions in case no suitable adapter
 is
  available. It rather is about the fact, that today the adaptTo is a
  barrier for all kinds of exceptions. In some cases the adaptation fails
  for a specific reason (one example is Sling Models where injection
 fails,
  another one is the issue mentioned in
  https://issues.apache.org/jira/browse/SLING-2712 (ValueMap not
 supporting
  primitives)). Both would be valid use cases where the client would be
  interested to catch the exception here.
  
  On 01 Jul 2014, at 10:34, Carsten Ziegeler cziege...@apache.org
 wrote:
  
   Adding a new interface would require us to implement it all over the
  place
   and as Felix points out, client code would always need to check
 whether
  the
   new interface is implemented or not Having to methods, like
 hasAdapter
  and
   adaptOrThrow does not work very well as between the two calls things
  might
   have changed already: while hasAdapter returns true, the underlying
  factory
   gets unregistered before adaptOrThrow is called.
   In many use cases, the current pattern works fine - the client does
 not
   care whether an exception is thrown within the adaption - it just
 cares
   whether an object is returned or not. And there are valid use cases,
  where
   client code does different things whether the adaption works or not
  (e.g.
   the post servlet checks for adaptTo(Node) and then does additional
  things
   if the resource is backed up by a node.)
  
   I see the point that there are also use cases where it would be fine
 to
   simpy throw an exception if adaptTo is not successful. This would
 make
  the
   client code easier. However as this most properly is a runtime
  exception,
   client code can today just call a method on the object and end up
 with a
   NPE - having the same result :)
  
   This leaves us with use cases where the client code explicitely
 wants to
   catch the exception and then do something depending on the exception.
  Maybe
   we should just add something for this explicit use case instead of
  bloating
   the general adaptTo mechanism?
  
   Regards
   Carsten
  
  
  
  
   2014-07-01 9:44 GMT+02:00 Konrad Windszus konra...@gmx.de:
  
   Regarding 1) Having such a Result class would mean that all consumer
  would
   need to unwrap the exception first. So instead of being forced of
   implementing a null-check (as with the old solution) one would need
 to
   implement another check. I want to prevent such a burden to the
  consumers.
   Regarding 2) Since the client code knows on which object the
   adaptToOrThrow is called I don¹t get why an instanceof check is
  necessary.
   Whether this object implements Adaptable2 is known at compile-time,
 so
  you
   do have the full IDE-support here.
   Regarding 3) In that case I would no longer allow a null value to be
   returned. One drawback is, that all the null checks are no longer
  effective.
  
   IMHO solution 2) is the best. At the same time I would deprecate the
  old
   Adaptable, because I cannot come up with a real use-case where
  returning
   has an advantage. I would rather implement another method boolean
   hasAdapter(ClassAdapterType type) on the Adaptable2 interface.
   Regards,
   Konrad
  
   On 01 Jul 2014, at 09:07, Felix Meschberger fmesc...@adobe.com
  wrote:
  
   Hi
  
   There currently are two issues floating around dealing with the
  question
   of returning more information than just null from the
   Adaptable.adaptTo(Class) method:
   https://issues.apache.org/jira/browse/SLING-3714 and
   https://issues.apache.org/jira/browse/SLING-3709. I think these
  requests
   warrant some discussion on the list.
  
   Background: adaptTo can be implemented

Re: adaptTo and results ....

I like that approach. It is backwards-compatible and allows the developers to 
decide whether they want to check for null or to rely on exceptions.
The AdapterManagerImpl indeed would need to deal with such a parametrisation 
and in addition the javadocs would need to be adjusted to make it clear that 
AdapterFactories may throw RuntimeExceptions. 
Those exceptions should be caught by the AdapterManagerImpl when the 
RequireAdapter was not requested and in the other case just passed along.


On 01 Jul 2014, at 09:44, Bertrand Delacretaz bdelacre...@apache.org wrote:

 On Tue, Jul 1, 2014 at 9:41 AM, Bertrand Delacretaz
 bdelacre...@apache.org wrote:
 ...how about this:
 
  Foo f = someObject.adaptTo(RequireAdapter.for(Foo.class));
 
 Actually, rereading SLING-3714, this can be made simpler with generics
 
  Foo f = someObject.adaptTo(RequireAdapterFoo.class));
 
 where RequireAdapter causes AdapterManagerImpl to wrap the adapters to
 throw an exception if adaption returns null.
 
 -Bertrand

RE: adaptTo and results ....

2014-07-01 Thread Stefan Seifert

 Foo f = someObject.adaptTo(RequireAdapterFoo.class));

this would still require an unwrapping of the object out of the 
RequireAdapterFoo instance.

 Foo f = someObject.adaptTo(RequireAdapter.for(Foo.class));

this looks interesting, and does not need unwrapping if the return value is the 
input class.
i assume it could be implemented using a ThreadLocal or similar as well?

stefan


-Original Message-
From: Konrad Windszus [mailto:konra...@gmx.de]
Sent: Tuesday, July 01, 2014 11:58 AM
To: dev@sling.apache.org
Cc: Bertrand Delacretaz
Subject: Re: adaptTo and results 

I like that approach. It is backwards-compatible and allows the developers to
decide whether they want to check for null or to rely on exceptions.
The AdapterManagerImpl indeed would need to deal with such a parametrisation
and in addition the javadocs would need to be adjusted to make it clear that
AdapterFactories may throw RuntimeExceptions.
Those exceptions should be caught by the AdapterManagerImpl when the
RequireAdapter was not requested and in the other case just passed along.


On 01 Jul 2014, at 09:44, Bertrand Delacretaz bdelacre...@apache.org wrote:

 On Tue, Jul 1, 2014 at 9:41 AM, Bertrand Delacretaz
 bdelacre...@apache.org wrote:
 ...how about this:

  Foo f = someObject.adaptTo(RequireAdapter.for(Foo.class));

 Actually, rereading SLING-3714, this can be made simpler with generics

  Foo f = someObject.adaptTo(RequireAdapterFoo.class));

 where RequireAdapter causes AdapterManagerImpl to wrap the adapters to
 throw an exception if adaption returns null.

 -Bertrand

Re: adaptTo and results ....

Hi Carsten,

Sure, but Konrad has a point in that I think sometimes the client *does*
care why the adaption failed.  For instance, if it had to do with
something entirely different from whether or not adaption would normally
work.

Let's say that I have a resource that should adapt to XYZ, but that my
adapter is currently buggy.  I'd like to get an exception for that, but
said exception is going to get eaten.

I do agree that if I have a resource that should NOT adapt to XYZ, that
getting back null is fine, and that I don't care why the adaption failed.

Cheers,
Jeff.


On 01/07/2014 10:19, Carsten Ziegeler cziege...@apache.org wrote:

Sure :) For the adapter pattern, the client does not care why the adaption
failed, the client is just interested in the result (success or not)
Validation is a different beast, if validation fails you want to know
specific reasons why it failed - and this can be multiple.
I tried to explain in my first mail on this thread, that all other use
cases mentioned can be handled with the current implementation - with the
exception of validation. But I think validation requires a different
concept than the adapter pattern.

Carsten


2014-07-01 11:09 GMT+02:00 Jeff Young j...@adobe.com:

 Hi Carsten,

 Can you say more?  (I'm not sure I understand what you're getting
at)

 Thanks,
 Jeff.


 On 01/07/2014 09:56, Carsten Ziegeler cziege...@apache.org wrote:

 adaption and validation are different concerns
 
 Carsten
 
 
 2014-07-01 10:55 GMT+02:00 Jeff Young j...@adobe.com:
 
  We could solve that by defining a specific exception for
  adaptation-not-possible and then catch only that.
 
  Of course that would leak tons of exceptions from code written before
 that
  exception became available.  Maybe do the catching based on some
sort of
  version clue?
 
  Cheers,
  Jeff.
 
 
  On 01/07/2014 09:40, Konrad Windszus konra...@gmx.de wrote:
 
  It is not (only) about throwing exceptions in case no suitable
adapter
 is
  available. It rather is about the fact, that today the adaptTo is a
  barrier for all kinds of exceptions. In some cases the adaptation
fails
  for a specific reason (one example is Sling Models where injection
 fails,
  another one is the issue mentioned in
  https://issues.apache.org/jira/browse/SLING-2712 (ValueMap not
 supporting
  primitives)). Both would be valid use cases where the client would
be
  interested to catch the exception here.
  
  On 01 Jul 2014, at 10:34, Carsten Ziegeler cziege...@apache.org
 wrote:
  
   Adding a new interface would require us to implement it all over
the
  place
   and as Felix points out, client code would always need to check
 whether
  the
   new interface is implemented or not Having to methods, like
 hasAdapter
  and
   adaptOrThrow does not work very well as between the two calls
things
  might
   have changed already: while hasAdapter returns true, the
underlying
  factory
   gets unregistered before adaptOrThrow is called.
   In many use cases, the current pattern works fine - the client
does
 not
   care whether an exception is thrown within the adaption - it just
 cares
   whether an object is returned or not. And there are valid use
cases,
  where
   client code does different things whether the adaption works or
not
  (e.g.
   the post servlet checks for adaptTo(Node) and then does additional
  things
   if the resource is backed up by a node.)
  
   I see the point that there are also use cases where it would be
fine
 to
   simpy throw an exception if adaptTo is not successful. This would
 make
  the
   client code easier. However as this most properly is a runtime
  exception,
   client code can today just call a method on the object and end up
 with a
   NPE - having the same result :)
  
   This leaves us with use cases where the client code explicitely
 wants to
   catch the exception and then do something depending on the
exception.
  Maybe
   we should just add something for this explicit use case instead of
  bloating
   the general adaptTo mechanism?
  
   Regards
   Carsten
  
  
  
  
   2014-07-01 9:44 GMT+02:00 Konrad Windszus konra...@gmx.de:
  
   Regarding 1) Having such a Result class would mean that all
consumer
  would
   need to unwrap the exception first. So instead of being forced of
   implementing a null-check (as with the old solution) one would
need
 to
   implement another check. I want to prevent such a burden to the
  consumers.
   Regarding 2) Since the client code knows on which object the
   adaptToOrThrow is called I don¹t get why an instanceof check is
  necessary.
   Whether this object implements Adaptable2 is known at
compile-time,
 so
  you
   do have the full IDE-support here.
   Regarding 3) In that case I would no longer allow a null value
to be
   returned. One drawback is, that all the null checks are no longer
  effective.
  
   IMHO solution 2) is the best. At the same time I would deprecate
the
  old
   Adaptable, because I cannot come up with a real use-case where

Re: adaptTo and results ....


On 01 Jul 2014, at 12:05, Stefan Seifert sseif...@pro-vision.de wrote:

 Foo f = someObject.adaptTo(RequireAdapterFoo.class));
 
 this would still require an unwrapping of the object out of the 
 RequireAdapterFoo instance.
In my regard there is an instanceof RequireAdapter check within the 
AdapterManagerImpl which would in that case just pass/throw exceptions. So no 
need to unwrap anything for the client.
The only questions is how to get the generic type at runtime (within the 
AdapterManagerImpl), but there are solutions to that as well: 
http://stackoverflow.com/questions/3403909/get-generic-type-of-class-at-runtime


 
 Foo f = someObject.adaptTo(RequireAdapter.for(Foo.class));
 
 this looks interesting, and does not need unwrapping if the return value is 
 the input class.
 i assume it could be implemented using a ThreadLocal or similar as well?
 
 stefan
 
 
 -Original Message-
 From: Konrad Windszus [mailto:konra...@gmx.de]
 Sent: Tuesday, July 01, 2014 11:58 AM
 To: dev@sling.apache.org
 Cc: Bertrand Delacretaz
 Subject: Re: adaptTo and results 
 
 I like that approach. It is backwards-compatible and allows the developers to
 decide whether they want to check for null or to rely on exceptions.
 The AdapterManagerImpl indeed would need to deal with such a parametrisation
 and in addition the javadocs would need to be adjusted to make it clear that
 AdapterFactories may throw RuntimeExceptions.
 Those exceptions should be caught by the AdapterManagerImpl when the
 RequireAdapter was not requested and in the other case just passed along.
 
 
 On 01 Jul 2014, at 09:44, Bertrand Delacretaz bdelacre...@apache.org wrote:
 
 On Tue, Jul 1, 2014 at 9:41 AM, Bertrand Delacretaz
 bdelacre...@apache.org wrote:
 ...how about this:
 
 Foo f = someObject.adaptTo(RequireAdapter.for(Foo.class));
 
 Actually, rereading SLING-3714, this can be made simpler with generics
 
 Foo f = someObject.adaptTo(RequireAdapterFoo.class));
 
 where RequireAdapter causes AdapterManagerImpl to wrap the adapters to
 throw an exception if adaption returns null.
 
 -Bertrand

Re: adaptTo and results ....

So if your adapter is buggy and you get an exception, what do you do with
it?

Carsten


2014-07-01 12:08 GMT+02:00 Jeff Young j...@adobe.com:

 Hi Carsten,

 Sure, but Konrad has a point in that I think sometimes the client *does*
 care why the adaption failed.  For instance, if it had to do with
 something entirely different from whether or not adaption would normally
 work.

 Let's say that I have a resource that should adapt to XYZ, but that my
 adapter is currently buggy.  I'd like to get an exception for that, but
 said exception is going to get eaten.

 I do agree that if I have a resource that should NOT adapt to XYZ, that
 getting back null is fine, and that I don't care why the adaption failed.

 Cheers,
 Jeff.


 On 01/07/2014 10:19, Carsten Ziegeler cziege...@apache.org wrote:

 Sure :) For the adapter pattern, the client does not care why the adaption
 failed, the client is just interested in the result (success or not)
 Validation is a different beast, if validation fails you want to know
 specific reasons why it failed - and this can be multiple.
 I tried to explain in my first mail on this thread, that all other use
 cases mentioned can be handled with the current implementation - with the
 exception of validation. But I think validation requires a different
 concept than the adapter pattern.
 
 Carsten
 
 
 2014-07-01 11:09 GMT+02:00 Jeff Young j...@adobe.com:
 
  Hi Carsten,
 
  Can you say more?  (I'm not sure I understand what you're getting
 at)
 
  Thanks,
  Jeff.
 
 
  On 01/07/2014 09:56, Carsten Ziegeler cziege...@apache.org wrote:
 
  adaption and validation are different concerns
  
  Carsten
  
  
  2014-07-01 10:55 GMT+02:00 Jeff Young j...@adobe.com:
  
   We could solve that by defining a specific exception for
   adaptation-not-possible and then catch only that.
  
   Of course that would leak tons of exceptions from code written before
  that
   exception became available.  Maybe do the catching based on some
 sort of
   version clue?
  
   Cheers,
   Jeff.
  
  
   On 01/07/2014 09:40, Konrad Windszus konra...@gmx.de wrote:
  
   It is not (only) about throwing exceptions in case no suitable
 adapter
  is
   available. It rather is about the fact, that today the adaptTo is a
   barrier for all kinds of exceptions. In some cases the adaptation
 fails
   for a specific reason (one example is Sling Models where injection
  fails,
   another one is the issue mentioned in
   https://issues.apache.org/jira/browse/SLING-2712 (ValueMap not
  supporting
   primitives)). Both would be valid use cases where the client would
 be
   interested to catch the exception here.
   
   On 01 Jul 2014, at 10:34, Carsten Ziegeler cziege...@apache.org
  wrote:
   
Adding a new interface would require us to implement it all over
 the
   place
and as Felix points out, client code would always need to check
  whether
   the
new interface is implemented or not Having to methods, like
  hasAdapter
   and
adaptOrThrow does not work very well as between the two calls
 things
   might
have changed already: while hasAdapter returns true, the
 underlying
   factory
gets unregistered before adaptOrThrow is called.
In many use cases, the current pattern works fine - the client
 does
  not
care whether an exception is thrown within the adaption - it just
  cares
whether an object is returned or not. And there are valid use
 cases,
   where
client code does different things whether the adaption works or
 not
   (e.g.
the post servlet checks for adaptTo(Node) and then does additional
   things
if the resource is backed up by a node.)
   
I see the point that there are also use cases where it would be
 fine
  to
simpy throw an exception if adaptTo is not successful. This would
  make
   the
client code easier. However as this most properly is a runtime
   exception,
client code can today just call a method on the object and end up
  with a
NPE - having the same result :)
   
This leaves us with use cases where the client code explicitely
  wants to
catch the exception and then do something depending on the
 exception.
   Maybe
we should just add something for this explicit use case instead of
   bloating
the general adaptTo mechanism?
   
Regards
Carsten
   
   
   
   
2014-07-01 9:44 GMT+02:00 Konrad Windszus konra...@gmx.de:
   
Regarding 1) Having such a Result class would mean that all
 consumer
   would
need to unwrap the exception first. So instead of being forced of
implementing a null-check (as with the old solution) one would
 need
  to
implement another check. I want to prevent such a burden to the
   consumers.
Regarding 2) Since the client code knows on which object the
adaptToOrThrow is called I don¹t get why an instanceof check is
   necessary.
Whether this object implements Adaptable2 is known at
 compile-time,
  so
   you
do have the full IDE-support here.

RE: adaptTo and results ....

2014-07-01 Thread Stefan Seifert

example: usecase like here
https://issues.apache.org/jira/browse/SLING-3714?focusedCommentId=14048040#comment-14048040

the caller code expects that the adaption is always successful if everything 
works correct - if not it is an application error which should be propagated 
through error handling and result in an error log message.

stefan


-Original Message-
From: Carsten Ziegeler [mailto:cziege...@apache.org]
Sent: Tuesday, July 01, 2014 12:14 PM
To: dev@sling.apache.org
Subject: Re: adaptTo and results 

So if your adapter is buggy and you get an exception, what do you do with
it?

Carsten


2014-07-01 12:08 GMT+02:00 Jeff Young j...@adobe.com:

 Hi Carsten,

 Sure, but Konrad has a point in that I think sometimes the client *does*
 care why the adaption failed.  For instance, if it had to do with
 something entirely different from whether or not adaption would normally
 work.

 Let's say that I have a resource that should adapt to XYZ, but that my
 adapter is currently buggy.  I'd like to get an exception for that, but
 said exception is going to get eaten.

 I do agree that if I have a resource that should NOT adapt to XYZ, that
 getting back null is fine, and that I don't care why the adaption failed.

 Cheers,
 Jeff.


 On 01/07/2014 10:19, Carsten Ziegeler cziege...@apache.org wrote:

 Sure :) For the adapter pattern, the client does not care why the adaption
 failed, the client is just interested in the result (success or not)
 Validation is a different beast, if validation fails you want to know
 specific reasons why it failed - and this can be multiple.
 I tried to explain in my first mail on this thread, that all other use
 cases mentioned can be handled with the current implementation - with the
 exception of validation. But I think validation requires a different
 concept than the adapter pattern.
 
 Carsten
 
 
 2014-07-01 11:09 GMT+02:00 Jeff Young j...@adobe.com:
 
  Hi Carsten,
 
  Can you say more?  (I'm not sure I understand what you're getting
 at)
 
  Thanks,
  Jeff.
 
 
  On 01/07/2014 09:56, Carsten Ziegeler cziege...@apache.org wrote:
 
  adaption and validation are different concerns
  
  Carsten
  
  
  2014-07-01 10:55 GMT+02:00 Jeff Young j...@adobe.com:
  
   We could solve that by defining a specific exception for
   adaptation-not-possible and then catch only that.
  
   Of course that would leak tons of exceptions from code written before
  that
   exception became available.  Maybe do the catching based on some
 sort of
   version clue?
  
   Cheers,
   Jeff.
  
  
   On 01/07/2014 09:40, Konrad Windszus konra...@gmx.de wrote:
  
   It is not (only) about throwing exceptions in case no suitable
 adapter
  is
   available. It rather is about the fact, that today the adaptTo is a
   barrier for all kinds of exceptions. In some cases the adaptation
 fails
   for a specific reason (one example is Sling Models where injection
  fails,
   another one is the issue mentioned in
   https://issues.apache.org/jira/browse/SLING-2712 (ValueMap not
  supporting
   primitives)). Both would be valid use cases where the client would
 be
   interested to catch the exception here.
   
   On 01 Jul 2014, at 10:34, Carsten Ziegeler cziege...@apache.org
  wrote:
   
Adding a new interface would require us to implement it all over
 the
   place
and as Felix points out, client code would always need to check
  whether
   the
new interface is implemented or not Having to methods, like
  hasAdapter
   and
adaptOrThrow does not work very well as between the two calls
 things
   might
have changed already: while hasAdapter returns true, the
 underlying
   factory
gets unregistered before adaptOrThrow is called.
In many use cases, the current pattern works fine - the client
 does
  not
care whether an exception is thrown within the adaption - it just
  cares
whether an object is returned or not. And there are valid use
 cases,
   where
client code does different things whether the adaption works or
 not
   (e.g.
the post servlet checks for adaptTo(Node) and then does additional
   things
if the resource is backed up by a node.)
   
I see the point that there are also use cases where it would be
 fine
  to
simpy throw an exception if adaptTo is not successful. This would
  make
   the
client code easier. However as this most properly is a runtime
   exception,
client code can today just call a method on the object and end up
  with a
NPE - having the same result :)
   
This leaves us with use cases where the client code explicitely
  wants to
catch the exception and then do something depending on the
 exception.
   Maybe
we should just add something for this explicit use case instead of
   bloating
the general adaptTo mechanism?
   
Regards
Carsten
   
   
   
   
2014-07-01 9:44 GMT+02:00 Konrad Windszus konra...@gmx.de:
   
Regarding 1) Having such a Result class would mean that all

Yes, but right now you would get an NPE accessing the object - so you
already have a runtime exception and don't need to check for null (I'm not
arguing that this is a good way, I'm just trying to avoid heavy changes).
And we could change the adapter manager/factory implemntation to log the
exceptions (if they're not doing it already)

Carsten

2014-07-01 12:17 GMT+02:00 Stefan Seifert sseif...@pro-vision.de:

example: usecase like here

https://issues.apache.org/jira/browse/SLING-3714?focusedCommentId=14048040#comment-14048040

the caller code expects that the adaption is always successful if
everything works correct - if not it is an application error which should
be propagated through error handling and result in an error log message.

stefan

-Original Message-
From: Carsten Ziegeler [mailto:cziege...@apache.org]
Sent: Tuesday, July 01, 2014 12:14 PM
To: dev@sling.apache.org
Subject: Re: adaptTo and results

So if your adapter is buggy and you get an exception, what do you do with
it?

Carsten

2014-07-01 12:08 GMT+02:00 Jeff Young j...@adobe.com:

Hi Carsten,

Sure, but Konrad has a point in that I think sometimes the client *does*
care why the adaption failed. For instance, if it had to do with
something entirely different from whether or not adaption would normally
work.

Let's say that I have a resource that should adapt to XYZ, but that my
adapter is currently buggy. I'd like to get an exception for that, but
said exception is going to get eaten.

I do agree that if I have a resource that should NOT adapt to XYZ, that
getting back null is fine, and that I don't care why the adaption
failed.

Cheers,
Jeff.

On 01/07/2014 10:19, Carsten Ziegeler cziege...@apache.org wrote:

Sure :) For the adapter pattern, the client does not care why the
adaption
failed, the client is just interested in the result (success or not)
Validation is a different beast, if validation fails you want to know
specific reasons why it failed - and this can be multiple.
I tried to explain in my first mail on this thread, that all other use
cases mentioned can be handled with the current implementation - with
the
exception of validation. But I think validation requires a different
concept than the adapter pattern.

Carsten

2014-07-01 11:09 GMT+02:00 Jeff Young j...@adobe.com:

Hi Carsten,

Can you say more? (I'm not sure I understand what you're getting
at)

Thanks,
Jeff.

On 01/07/2014 09:56, Carsten Ziegeler cziege...@apache.org
wrote:

adaption and validation are different concerns

Carsten

2014-07-01 10:55 GMT+02:00 Jeff Young j...@adobe.com:

We could solve that by defining a specific exception for
adaptation-not-possible and then catch only that.

Of course that would leak tons of exceptions from code written
before
that
exception became available. Maybe do the catching based on some
sort of
version clue?

Cheers,
Jeff.

On 01/07/2014 09:40, Konrad Windszus konra...@gmx.de wrote:

It is not (only) about throwing exceptions in case no suitable
adapter
is
available. It rather is about the fact, that today the adaptTo
is a
barrier for all kinds of exceptions. In some cases the adaptation
fails
for a specific reason (one example is Sling Models where
injection
fails,
another one is the issue mentioned in
https://issues.apache.org/jira/browse/SLING-2712 (ValueMap not
supporting
primitives)). Both would be valid use cases where the client
would
be
interested to catch the exception here.

On 01 Jul 2014, at 10:34, Carsten Ziegeler cziege...@apache.org

wrote:

Adding a new interface would require us to implement it all
over
the
place
and as Felix points out, client code would always need to check
whether
the
new interface is implemented or not Having to methods, like
hasAdapter
and
adaptOrThrow does not work very well as between the two calls
things
might
have changed already: while hasAdapter returns true, the
underlying
factory
gets unregistered before adaptOrThrow is called.
In many use cases, the current pattern works fine - the client
does
not
care whether an exception is thrown within the adaption - it
just
cares
whether an object is returned or not. And there are valid use
cases,
where
client code does different things whether the adaption works or
not
(e.g.
the post servlet checks for adaptTo(Node) and then does
additional
things
if the resource is backed up by a node.)

Re: adaptTo and results ....

I just fix it in the code ;-). Those exceptions should only happen during
runtime (due to some false assumptions).
For the same reasons methods do throw IllegalArgumentExceptions in case a given
parameter is null
(http://stackoverflow.com/questions/5172948/should-we-always-check-each-parameter-of-method-in-java-for-null-in-the-first-li).
This is mainly for the developer, but makes the life much easier as with that
information it is obvious how to fix :-)
Konrad

On 01 Jul 2014, at 12:14, Carsten Ziegeler cziege...@apache.org wrote:

So if your adapter is buggy and you get an exception, what do you do with
it?

Carsten

2014-07-01 12:08 GMT+02:00 Jeff Young j...@adobe.com:

Hi Carsten,

Let's say that I have a resource that should adapt to XYZ, but that my
adapter is currently buggy. I'd like to get an exception for that, but
said exception is going to get eaten.

I do agree that if I have a resource that should NOT adapt to XYZ, that
getting back null is fine, and that I don't care why the adaption failed.

Cheers,
Jeff.

On 01/07/2014 10:19, Carsten Ziegeler cziege...@apache.org wrote:

Sure :) For the adapter pattern, the client does not care why the adaption
failed, the client is just interested in the result (success or not)
Validation is a different beast, if validation fails you want to know
specific reasons why it failed - and this can be multiple.
I tried to explain in my first mail on this thread, that all other use
cases mentioned can be handled with the current implementation - with the
exception of validation. But I think validation requires a different
concept than the adapter pattern.

Carsten

2014-07-01 11:09 GMT+02:00 Jeff Young j...@adobe.com:

Hi Carsten,

Can you say more? (I'm not sure I understand what you're getting
at)

Thanks,
Jeff.

On 01/07/2014 09:56, Carsten Ziegeler cziege...@apache.org wrote:

adaption and validation are different concerns

Carsten

2014-07-01 10:55 GMT+02:00 Jeff Young j...@adobe.com:

We could solve that by defining a specific exception for
adaptation-not-possible and then catch only that.

Of course that would leak tons of exceptions from code written before
that
exception became available. Maybe do the catching based on some
sort of
version clue?

Cheers,
Jeff.

On 01/07/2014 09:40, Konrad Windszus konra...@gmx.de wrote:

It is not (only) about throwing exceptions in case no suitable
adapter
is
available. It rather is about the fact, that today the adaptTo is a
barrier for all kinds of exceptions. In some cases the adaptation
fails
for a specific reason (one example is Sling Models where injection
fails,
another one is the issue mentioned in
https://issues.apache.org/jira/browse/SLING-2712 (ValueMap not
supporting
primitives)). Both would be valid use cases where the client would
be
interested to catch the exception here.

On 01 Jul 2014, at 10:34, Carsten Ziegeler cziege...@apache.org
wrote:

Adding a new interface would require us to implement it all over
the
place
and as Felix points out, client code would always need to check
whether
the
new interface is implemented or not Having to methods, like
hasAdapter
and
adaptOrThrow does not work very well as between the two calls
things
might
have changed already: while hasAdapter returns true, the
underlying
factory
gets unregistered before adaptOrThrow is called.
In many use cases, the current pattern works fine - the client
does
not
care whether an exception is thrown within the adaption - it just
cares
whether an object is returned or not. And there are valid use
cases,
where
client code does different things whether the adaption works or
not
(e.g.
the post servlet checks for adaptTo(Node) and then does additional
things
if the resource is backed up by a node.)

I see the point that there are also use cases where it would be
fine
to
simpy throw an exception if adaptTo is not successful. This would
make
the
client code easier. However as this most properly is a runtime
exception,
client code can today just call a method on the object and end up
with a
NPE - having the same result :)

This leaves us with use cases where the client code explicitely
wants to
catch the exception and then do something depending on the
exception.
Maybe
we should just add something for this explicit use case instead of
bloating
the general adaptTo mechanism?

Regards
Carsten

2014-07-01 9:44 GMT+02:00 Konrad Windszus konra...@gmx.de:

Regarding 1) Having such a Result class would mean that all
consumer
would
need to unwrap the exception first. So instead of being forced of
implementing a null-check (as with the old solution) one would

RE: adaptTo and results ....

2014-07-01 Thread Stefan Seifert

the NPE would swallow all maybe usefull excpetion information, that might be 
contained in the root cause of the exception throws by a method like 
adaptToOrThrow method. always logging the exception internally by the adapter 
manager has the drawback that the application might not be interested in the 
failure and does not want to log it. the decision whether a adaption failure is 
relevant or not should be taken by the application.

I'm not convinced that a new interface and a adaptToOrThrow is the best 
solution either - but lets start to convince ourselves that it is a relevant 
usecase to have (optional, but with full error information) exception handling 
on an adaptTo call, whatever solution we find to add this without a big mess in 
the interface design. bertrand opened an interesting discussion on alternatives.

stefan

-Original Message-
From: Carsten Ziegeler [mailto:cziege...@apache.org]
Sent: Tuesday, July 01, 2014 12:21 PM
To: dev@sling.apache.org
Subject: Re: adaptTo and results 

Yes, but right now you would get an NPE accessing the object - so you
already have a runtime exception and don't need to check for null (I'm not
arguing that this is a good way, I'm just trying to avoid heavy changes).
And we could change the adapter manager/factory implemntation to log the
exceptions (if they're not doing it already)

Carsten


2014-07-01 12:17 GMT+02:00 Stefan Seifert sseif...@pro-vision.de:

 example: usecase like here

 https://issues.apache.org/jira/browse/SLING-
3714?focusedCommentId=14048040#comment-14048040

 the caller code expects that the adaption is always successful if
 everything works correct - if not it is an application error which should
 be propagated through error handling and result in an error log message.

 stefan


 -Original Message-
 From: Carsten Ziegeler [mailto:cziege...@apache.org]
 Sent: Tuesday, July 01, 2014 12:14 PM
 To: dev@sling.apache.org
 Subject: Re: adaptTo and results 
 
 So if your adapter is buggy and you get an exception, what do you do with
 it?
 
 Carsten
 
 
 2014-07-01 12:08 GMT+02:00 Jeff Young j...@adobe.com:
 
  Hi Carsten,
 
  Sure, but Konrad has a point in that I think sometimes the client *does*
  care why the adaption failed.  For instance, if it had to do with
  something entirely different from whether or not adaption would normally
  work.
 
  Let's say that I have a resource that should adapt to XYZ, but that my
  adapter is currently buggy.  I'd like to get an exception for that, but
  said exception is going to get eaten.
 
  I do agree that if I have a resource that should NOT adapt to XYZ, that
  getting back null is fine, and that I don't care why the adaption
 failed.
 
  Cheers,
  Jeff.
 
 
  On 01/07/2014 10:19, Carsten Ziegeler cziege...@apache.org wrote:
 
  Sure :) For the adapter pattern, the client does not care why the
 adaption
  failed, the client is just interested in the result (success or not)
  Validation is a different beast, if validation fails you want to know
  specific reasons why it failed - and this can be multiple.
  I tried to explain in my first mail on this thread, that all other use
  cases mentioned can be handled with the current implementation - with
 the
  exception of validation. But I think validation requires a different
  concept than the adapter pattern.
  
  Carsten
  
  
  2014-07-01 11:09 GMT+02:00 Jeff Young j...@adobe.com:
  
   Hi Carsten,
  
   Can you say more?  (I'm not sure I understand what you're getting
  at)
  
   Thanks,
   Jeff.
  
  
   On 01/07/2014 09:56, Carsten Ziegeler cziege...@apache.org
 wrote:
  
   adaption and validation are different concerns
   
   Carsten
   
   
   2014-07-01 10:55 GMT+02:00 Jeff Young j...@adobe.com:
   
We could solve that by defining a specific exception for
adaptation-not-possible and then catch only that.
   
Of course that would leak tons of exceptions from code written
 before
   that
exception became available.  Maybe do the catching based on some
  sort of
version clue?
   
Cheers,
Jeff.
   
   
On 01/07/2014 09:40, Konrad Windszus konra...@gmx.de wrote:
   
It is not (only) about throwing exceptions in case no suitable
  adapter
   is
available. It rather is about the fact, that today the adaptTo
 is a
barrier for all kinds of exceptions. In some cases the adaptation
  fails
for a specific reason (one example is Sling Models where
 injection
   fails,
another one is the issue mentioned in
https://issues.apache.org/jira/browse/SLING-2712 (ValueMap not
   supporting
primitives)). Both would be valid use cases where the client
 would
  be
interested to catch the exception here.

On 01 Jul 2014, at 10:34, Carsten Ziegeler cziege...@apache.org
 
   wrote:

 Adding a new interface would require us to implement it all
 over
  the
place
 and as Felix points out, client code would always need to check
   whether
the
 new

Re: adaptTo and results ....

2014-07-01 Thread Stefan Egli

I like the idea too, but I guess it's merely a question of taste as to
which of the following two options is nicer:
 * Foo f = someObject.adaptTo(RequireAdapterFoo.class));
 * Foo f = someObject.adaptToUnchecked(Foo.class);

Cheers,
Stefan

On 7/1/14 11:57 AM, Konrad Windszus konra...@gmx.de wrote:

I like that approach. It is backwards-compatible and allows the
developers to decide whether they want to check for null or to rely on
exceptions.
The AdapterManagerImpl indeed would need to deal with such a
parametrisation and in addition the javadocs would need to be adjusted to
make it clear that AdapterFactories may throw RuntimeExceptions.
Those exceptions should be caught by the AdapterManagerImpl when the
RequireAdapter was not requested and in the other case just passed along.


On 01 Jul 2014, at 09:44, Bertrand Delacretaz bdelacre...@apache.org
wrote:

 On Tue, Jul 1, 2014 at 9:41 AM, Bertrand Delacretaz
 bdelacre...@apache.org wrote:
 ...how about this:
 
  Foo f = someObject.adaptTo(RequireAdapter.for(Foo.class));
 
 Actually, rereading SLING-3714, this can be made simpler with generics
 
  Foo f = someObject.adaptTo(RequireAdapterFoo.class));
 
 where RequireAdapter causes AdapterManagerImpl to wrap the adapters to
 throw an exception if adaption returns null.
 
 -Bertrand

Re: adaptTo and results ....

2014-07-01 Thread Bertrand Delacretaz

On Tue, Jul 1, 2014 at 12:38 PM, Stefan Egli stefane...@apache.org wrote:
 I like the idea too, but I guess it's merely a question of taste as to
 which of the following two options is nicer:
  * Foo f = someObject.adaptTo(RequireAdapterFoo.class));
  * Foo f = someObject.adaptToUnchecked(Foo.class);

The big difference is that the first variant requires no API changes
and only requires code changes in AdapterManagerImpl (I think -
haven't looked in full detail ;-)

-Bertrand

Re: adaptTo and results ....

Ok, this would solve the throw if adaption is not possible case, what about
the validation use case?

Carsten


2014-07-01 12:50 GMT+02:00 Bertrand Delacretaz bdelacre...@apache.org:

 On Tue, Jul 1, 2014 at 12:38 PM, Stefan Egli stefane...@apache.org
 wrote:
  I like the idea too, but I guess it's merely a question of taste as to
  which of the following two options is nicer:
   * Foo f = someObject.adaptTo(RequireAdapterFoo.class));
   * Foo f = someObject.adaptToUnchecked(Foo.class);

 The big difference is that the first variant requires no API changes
 and only requires code changes in AdapterManagerImpl (I think -
 haven't looked in full detail ;-)

 -Bertrand




-- 
Carsten Ziegeler
Adobe Research Switzerland
cziege...@apache.org

Re: adaptTo and results ....

That would be solved by just stating that RuntimeExceptions are allowed as 
alternative to returning null for all AdapterFactories (i.e. no API change 
necessary) and making sure that those exceptions are either being caught within 
the AdapterManagerImpl or just propagated to the caller.

On 01 Jul 2014, at 13:08, Carsten Ziegeler cziege...@apache.org wrote:

 Ok, this would solve the throw if adaption is not possible case, what about
 the validation use case?
 
 Carsten
 
 
 2014-07-01 12:50 GMT+02:00 Bertrand Delacretaz bdelacre...@apache.org:
 
 On Tue, Jul 1, 2014 at 12:38 PM, Stefan Egli stefane...@apache.org
 wrote:
 I like the idea too, but I guess it's merely a question of taste as to
 which of the following two options is nicer:
 * Foo f = someObject.adaptTo(RequireAdapterFoo.class));
 * Foo f = someObject.adaptToUnchecked(Foo.class);
 
 The big difference is that the first variant requires no API changes
 and only requires code changes in AdapterManagerImpl (I think -
 haven't looked in full detail ;-)
 
 -Bertrand
 
 
 
 
 -- 
 Carsten Ziegeler
 Adobe Research Switzerland
 cziege...@apache.org

Re: adaptTo and results ....

Well, for one thing, display it in the Developer Mode console (or whatever
other debugging UIs my app happens to have).

Jeff.


On 01/07/2014 11:14, Carsten Ziegeler cziege...@apache.org wrote:

So if your adapter is buggy and you get an exception, what do you do with
it?

Carsten


2014-07-01 12:08 GMT+02:00 Jeff Young j...@adobe.com:

 Hi Carsten,

 Sure, but Konrad has a point in that I think sometimes the client *does*
 care why the adaption failed.  For instance, if it had to do with
 something entirely different from whether or not adaption would normally
 work.

 Let's say that I have a resource that should adapt to XYZ, but that my
 adapter is currently buggy.  I'd like to get an exception for that, but
 said exception is going to get eaten.

 I do agree that if I have a resource that should NOT adapt to XYZ, that
 getting back null is fine, and that I don't care why the adaption
failed.

 Cheers,
 Jeff.


 On 01/07/2014 10:19, Carsten Ziegeler cziege...@apache.org wrote:

 Sure :) For the adapter pattern, the client does not care why the
adaption
 failed, the client is just interested in the result (success or not)
 Validation is a different beast, if validation fails you want to know
 specific reasons why it failed - and this can be multiple.
 I tried to explain in my first mail on this thread, that all other use
 cases mentioned can be handled with the current implementation - with
the
 exception of validation. But I think validation requires a different
 concept than the adapter pattern.
 
 Carsten
 
 
 2014-07-01 11:09 GMT+02:00 Jeff Young j...@adobe.com:
 
  Hi Carsten,
 
  Can you say more?  (I'm not sure I understand what you're getting
 at)
 
  Thanks,
  Jeff.
 
 
  On 01/07/2014 09:56, Carsten Ziegeler cziege...@apache.org wrote:
 
  adaption and validation are different concerns
  
  Carsten
  
  
  2014-07-01 10:55 GMT+02:00 Jeff Young j...@adobe.com:
  
   We could solve that by defining a specific exception for
   adaptation-not-possible and then catch only that.
  
   Of course that would leak tons of exceptions from code written
before
  that
   exception became available.  Maybe do the catching based on some
 sort of
   version clue?
  
   Cheers,
   Jeff.
  
  
   On 01/07/2014 09:40, Konrad Windszus konra...@gmx.de wrote:
  
   It is not (only) about throwing exceptions in case no suitable
 adapter
  is
   available. It rather is about the fact, that today the adaptTo
is a
   barrier for all kinds of exceptions. In some cases the adaptation
 fails
   for a specific reason (one example is Sling Models where
injection
  fails,
   another one is the issue mentioned in
   https://issues.apache.org/jira/browse/SLING-2712 (ValueMap not
  supporting
   primitives)). Both would be valid use cases where the client
would
 be
   interested to catch the exception here.
   
   On 01 Jul 2014, at 10:34, Carsten Ziegeler cziege...@apache.org
  wrote:
   
Adding a new interface would require us to implement it all
over
 the
   place
and as Felix points out, client code would always need to check
  whether
   the
new interface is implemented or not Having to methods, like
  hasAdapter
   and
adaptOrThrow does not work very well as between the two calls
 things
   might
have changed already: while hasAdapter returns true, the
 underlying
   factory
gets unregistered before adaptOrThrow is called.
In many use cases, the current pattern works fine - the client
 does
  not
care whether an exception is thrown within the adaption - it
just
  cares
whether an object is returned or not. And there are valid use
 cases,
   where
client code does different things whether the adaption works or
 not
   (e.g.
the post servlet checks for adaptTo(Node) and then does
additional
   things
if the resource is backed up by a node.)
   
I see the point that there are also use cases where it would be
 fine
  to
simpy throw an exception if adaptTo is not successful. This
would
  make
   the
client code easier. However as this most properly is a runtime
   exception,
client code can today just call a method on the object and end
up
  with a
NPE - having the same result :)
   
This leaves us with use cases where the client code explicitely
  wants to
catch the exception and then do something depending on the
 exception.
   Maybe
we should just add something for this explicit use case
instead of
   bloating
the general adaptTo mechanism?
   
Regards
Carsten
   
   
   
   
2014-07-01 9:44 GMT+02:00 Konrad Windszus konra...@gmx.de:
   
Regarding 1) Having such a Result class would mean that all
 consumer
   would
need to unwrap the exception first. So instead of being
forced of
implementing a null-check (as with the old solution) one would
 need
  to
implement another check. I want to prevent such a burden to
the
   consumers.
Regarding 2) Since the client code knows on which object the

Re: adaptTo and results ....