Exception causes swallowed in OslcQueryResult

Hi,

As RequirementCollection is no longer a Requirement I was trying to get the result members as IResources:

        for (IResource resource: result.getMembers(IResource.class)) {
            if (resource instanceof Requirement) {
                Requirement bean = (Requirement) resource;

Now I see this on the console:

 java.lang.IllegalStateException: org.eclipse.lyo.oslc.domains.am.IResource
	at org.eclipse.lyo.client.query.OslcQueryResult$1$1.next(OslcQueryResult.java:276)

Looking at OslcQueryResult.getMembers() I see this:

try {
  return (T)JenaModelHelper.fromJenaResource((Resource)member.getObject(), clazz);
} catch (IllegalArgumentException e) {
 throw new IllegalStateException(e.getMessage());
} catch (SecurityException e) {
  throw new IllegalStateException(e.getMessage());
} catch (DatatypeConfigurationException e) {
  throw new IllegalStateException(e.getMessage());
} catch (IllegalAccessException e) {
  throw new IllegalStateException(e.getMessage());
} catch (InstantiationException e) {
  throw new IllegalStateException(e.getMessage());
} catch (InvocationTargetException e) {
  throw new IllegalStateException(e.getMessage());
} catch (OslcCoreApplicationException e) {
  throw new IllegalStateException(e.getMessage());
} catch (URISyntaxException e) {
  throw new IllegalStateException(e.getMessage());
} catch (NoSuchMethodException e) {
  throw new IllegalStateException(e.getMessage());
}

What’s the reason for swallowing all the interesting original exceptions and pretending they’re all IllegalStateExceptions?

And why is JenaModelHelper.fromJenaResource() deprecated? Where is the replacement functionality? Note that you can give such valuable information to your users via a JavaDoc annotation.

Blanket (incorrect) wrapping of the exceptions is bad, let’s continue in

As to why this was done, my guess is that in the old days there was no strategy for dealing with exceptions and every checked exception was bubbled up, causing a huge catch statement on every call plus no clear message cause. The way I do it is to define a Lyo specific exception and then wrap the thrown exception in it if I understand what is the possible cause of the underlying exception. For example,

public static <T> T unmarshal(final Resource resource, Class<T> clazz) throws LyoModelException {
    try {
        return (T)JenaModelHelper.fromJenaResource(resource, clazz);
    } catch (DatatypeConfigurationException | IllegalAccessException |
            InvocationTargetException | InstantiationException | OslcCoreApplicationException
            | NoSuchMethodException | URISyntaxException e) {
        throw new LyoModelException(e);
    }
}

But the problem here is that neither a Lyo-specific exception is used nor the whole underlyig exception is wrapped, just its message.

Because of (a) having all those non-specific exceptions in the signature and (b) returning an object.

The unmarshal method shown above is the recommended replacement. If you have a better pattern in mind, we are glad to improve.

This is the signature of the deprecated method:

/**
 * @see #unmarshal(Resource, Class)
 * @see #unmarshalSingle(Model, Class)
 */
@Deprecated
public static Object fromJenaResource(final Resource resource, Class<?> beanClass)

The @see tag also seems to render well in the Javadoc: JenaModelHelper (Lyo :: _Parent 5.0.0-SNAPSHOT API) . Is there a better way?

I read @see specifications as part of the deprecated stuff, i.e., I don’t read them at all.

The @deprecated JavaDoc annotation is the recommended way of telling users where to go instead: https://docs.oracle.com/javase/7/docs/technotes/guides/javadoc/deprecation/deprecation.html

Here an example:

  /**
   * @since 4.2
   * @deprecated As of 4.6. use {@link #invalidate(ViewInvalidationData)}
   */
  @Deprecated
  public void invalidate(CDOBranch branch, long lastUpdateTime, List<CDORevisionKey> allChangedObjects, List<CDOIDAndVersion> allDetachedObjects,
      Map<CDOID, InternalCDORevision> oldRevisions, boolean async, boolean clearResourcePathCache);