From 0930abd89f504f1b374467af2311676bfc82c0d6 Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Wed, 06 Mar 2013 10:01:06 +0000
Subject: [PATCH] Minor improvements:

---
 opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/SimpleAttributeMapper.java    |   47 ++++++++---
 opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Utils.java                    |  111 ++++++++++++++++++++-------
 opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ReferenceAttributeMapper.java |   55 ++++++++-----
 3 files changed, 148 insertions(+), 65 deletions(-)

diff --git a/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ReferenceAttributeMapper.java b/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ReferenceAttributeMapper.java
index 02b6be7..7b52566 100644
--- a/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ReferenceAttributeMapper.java
+++ b/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ReferenceAttributeMapper.java
@@ -113,31 +113,42 @@
         if (attribute == null) {
             h.handleResult(null);
         } else if (attributeIsSingleValued()) {
-            final DN dn = attribute.parse().usingSchema(c.getConfig().schema()).asDN();
-            readEntry(c, dn, h);
+            try {
+                final DN dn = attribute.parse().usingSchema(c.getConfig().schema()).asDN();
+                readEntry(c, dn, h);
+            } catch (Exception ex) {
+                // The LDAP attribute could not be decoded.
+                h.handleError(adapt(ex));
+            }
         } else {
-            final Set<DN> dns = attribute.parse().usingSchema(c.getConfig().schema()).asSetOfDN();
-            final ResultHandler<JsonValue> handler =
-                    accumulate(dns.size(), transform(
-                            new Function<List<JsonValue>, JsonValue, Void>() {
-                                @Override
-                                public JsonValue apply(final List<JsonValue> value, final Void p) {
-                                    if (value.isEmpty()) {
-                                        // No values, so omit the entire JSON object from the resource.
-                                        return null;
-                                    } else {
-                                        // Combine values into a single JSON array.
-                                        final List<Object> result =
-                                                new ArrayList<Object>(value.size());
-                                        for (final JsonValue e : value) {
-                                            result.add(e.getObject());
+            try {
+                final Set<DN> dns =
+                        attribute.parse().usingSchema(c.getConfig().schema()).asSetOfDN();
+                final ResultHandler<JsonValue> handler =
+                        accumulate(dns.size(), transform(
+                                new Function<List<JsonValue>, JsonValue, Void>() {
+                                    @Override
+                                    public JsonValue apply(final List<JsonValue> value, final Void p) {
+                                        if (value.isEmpty()) {
+                                            // No values, so omit the entire JSON object from the resource.
+                                            return null;
+                                        } else {
+                                            // Combine values into a single JSON array.
+                                            final List<Object> result =
+                                                    new ArrayList<Object>(value.size());
+                                            for (final JsonValue e : value) {
+                                                result.add(e.getObject());
+                                            }
+                                            return new JsonValue(result);
                                         }
-                                        return new JsonValue(result);
                                     }
-                                }
-                            }, h));
-            for (final DN dn : dns) {
-                readEntry(c, dn, handler);
+                                }, h));
+                for (final DN dn : dns) {
+                    readEntry(c, dn, handler);
+                }
+            } catch (Exception ex) {
+                // The LDAP attribute could not be decoded.
+                h.handleError(adapt(ex));
             }
         }
     }
diff --git a/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/SimpleAttributeMapper.java b/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/SimpleAttributeMapper.java
index c014547..c1ddd3c 100644
--- a/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/SimpleAttributeMapper.java
+++ b/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/SimpleAttributeMapper.java
@@ -21,6 +21,7 @@
 import static java.util.Collections.singletonList;
 import static org.forgerock.opendj.ldap.Filter.alwaysFalse;
 import static org.forgerock.opendj.ldap.Functions.fixedFunction;
+import static org.forgerock.opendj.rest2ldap.Utils.adapt;
 import static org.forgerock.opendj.rest2ldap.Utils.base64ToByteString;
 import static org.forgerock.opendj.rest2ldap.Utils.byteStringToBase64;
 import static org.forgerock.opendj.rest2ldap.Utils.byteStringToJson;
@@ -181,7 +182,16 @@
     void getLDAPFilter(final Context c, final FilterType type, final JsonPointer jsonAttribute,
             final String operator, final Object valueAssertion, final ResultHandler<Filter> h) {
         if (jsonAttribute.isEmpty()) {
-            h.handleResult(toFilter(c, type, ldapAttributeName.toString(), valueAssertion));
+            try {
+                final ByteString va = encoder().apply(valueAssertion, null);
+                h.handleResult(toFilter(c, type, ldapAttributeName.toString(), va));
+            } catch (Exception e) {
+                // Invalid assertion value - bad request.
+
+                // FIXME: improve error message.
+                h.handleError(new BadRequestException("Invalid filter assertion value '"
+                        + String.valueOf(valueAssertion) + "'", e));
+            }
         } else {
             // This attribute mapper does not support partial filtering.
             h.handleResult(alwaysFalse());
@@ -190,16 +200,20 @@
 
     @Override
     void toJSON(final Context c, final Entry e, final ResultHandler<JsonValue> h) {
-        final Function<ByteString, ?, Void> f =
-                decoder == null ? fixedFunction(byteStringToJson(), ldapAttributeName) : decoder;
-        final Object value;
-        if (attributeIsSingleValued()) {
-            value = e.parseAttribute(ldapAttributeName).as(f, defaultJSONValue);
-        } else {
-            final Set<Object> s = e.parseAttribute(ldapAttributeName).asSetOf(f, defaultJSONValues);
-            value = s.isEmpty() ? null : s;
+        try {
+            final Object value;
+            if (attributeIsSingleValued()) {
+                value = e.parseAttribute(ldapAttributeName).as(decoder(), defaultJSONValue);
+            } else {
+                final Set<Object> s =
+                        e.parseAttribute(ldapAttributeName).asSetOf(decoder(), defaultJSONValues);
+                value = s.isEmpty() ? null : s;
+            }
+            h.handleResult(value != null ? new JsonValue(value) : null);
+        } catch (Exception ex) {
+            // The LDAP attribute could not be decoded.
+            h.handleError(adapt(ex));
         }
-        h.handleResult(value != null ? new JsonValue(value) : null);
     }
 
     @Override
@@ -230,10 +244,7 @@
             } else {
                 final Object value = v.getObject();
                 if (value != null) {
-                    final Function<Object, ByteString, Void> f =
-                            encoder != null ? encoder : fixedFunction(jsonToByteString(),
-                                    ldapAttributeName);
-                    result = singletonList(jsonToAttribute(value, ldapAttributeName, f));
+                    result = singletonList(jsonToAttribute(value, ldapAttributeName, encoder()));
                 } else if (defaultLDAPValue != null) {
                     result =
                             singletonList((Attribute) new LinkedAttribute(ldapAttributeName,
@@ -259,4 +270,12 @@
         return isSingleValued || ldapAttributeName.getAttributeType().isSingleValue();
     }
 
+    private Function<ByteString, ? extends Object, Void> decoder() {
+        return decoder == null ? fixedFunction(byteStringToJson(), ldapAttributeName) : decoder;
+    }
+
+    private Function<Object, ByteString, Void> encoder() {
+        return encoder == null ? fixedFunction(jsonToByteString(), ldapAttributeName) : encoder;
+    }
+
 }
diff --git a/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Utils.java b/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Utils.java
index ce51dcc..a39c966 100644
--- a/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Utils.java
+++ b/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Utils.java
@@ -68,6 +68,7 @@
         private final ResultHandler<List<V>> handler;
         private final AtomicInteger latch;
         private final List<V> results;
+        private ResourceException exception; // Guarded by latch.
 
         private AccumulatingResultHandler(final int size, final ResultHandler<List<V>> handler) {
             this.latch = new AtomicInteger(size);
@@ -75,20 +76,12 @@
             this.handler = handler;
         }
 
-        /**
-         * {@inheritDoc}
-         */
         @Override
         public void handleError(final ResourceException e) {
-            // Ensure that handler is only invoked once.
-            if (latch.getAndSet(0) > 0) {
-                handler.handleError(e);
-            }
+            exception = e;
+            latch(); // Volatile write publishes exception.
         }
 
-        /**
-         * {@inheritDoc}
-         */
         @Override
         public void handleResult(final V result) {
             if (result != null) {
@@ -96,11 +89,21 @@
                     results.add(result);
                 }
             }
-            if (latch.decrementAndGet() == 0) {
-                handler.handleResult(results);
-            }
+            latch();
         }
 
+        private void latch() {
+            // Invoke the handler once all results have been received. Avoid failing-fast
+            // when an error occurs because some in-flight tasks may depend on resources
+            // (e.g. connections) which are automatically closed on completion.
+            if (latch.decrementAndGet() == 0) {
+                if (exception != null) {
+                    handler.handleError(exception);
+                } else {
+                    handler.handleResult(results);
+                }
+            }
+        }
     }
 
     private static final Function<Object, ByteString, Void> BASE64_TO_BYTESTRING =
@@ -156,21 +159,45 @@
                 }
             };
 
+    /**
+     * Returns a result handler which can be used to collect the results of
+     * {@code size} asynchronous operations. Once all results have been received
+     * {@code handler} will be invoked with a list containing the results.
+     * Accumulation ignores {@code null} results, so the result list may be
+     * smaller than {@code size}. The returned result handler does not
+     * fail-fast: it will wait until all results have been received even if an
+     * error has been detected. This ensures that asynchronous operations can
+     * use resources such as connections which are automatically released
+     * (closed) upon completion of the final operation.
+     *
+     * @param <V>
+     *            The type of result to be collected.
+     * @param size
+     *            The number of expected results.
+     * @param handler
+     *            The result handler to be invoked when all results have been
+     *            received.
+     * @return A result handler which can be used to collect the results of
+     *         {@code size} asynchronous operations.
+     */
     static <V> ResultHandler<V> accumulate(final int size, final ResultHandler<List<V>> handler) {
         return new AccumulatingResultHandler<V>(size, handler);
     }
 
     /**
-     * Adapts an LDAP result code to a resource exception.
+     * Adapts a {@code Throwable} to a {@code ResourceException}. If the
+     * {@code Throwable} is an LDAP {@code ErrorResultException} then an
+     * appropriate {@code ResourceException} is returned, otherwise an
+     * {@code InternalServerErrorException} is returned.
      *
-     * @param error
-     *            The LDAP error that should be adapted.
+     * @param t
+     *            The {@code Throwable} to be converted.
      * @return The equivalent resource exception.
      */
-    static ResourceException adapt(final ErrorResultException error) {
+    static ResourceException adapt(final Throwable t) {
         int resourceResultCode;
         try {
-            throw error;
+            throw t;
         } catch (final AssertionFailureException e) {
             resourceResultCode = ResourceException.VERSION_MISMATCH;
         } catch (final AuthenticationException e) {
@@ -194,8 +221,10 @@
             } else {
                 resourceResultCode = ResourceException.INTERNAL_ERROR;
             }
+        } catch (final Throwable tmp) {
+            resourceResultCode = ResourceException.INTERNAL_ERROR;
         }
-        return ResourceException.getException(resourceResultCode, error.getMessage(), error);
+        return ResourceException.getException(resourceResultCode, t.getMessage(), t);
     }
 
     static Object attributeToJson(final Attribute a) {
@@ -269,30 +298,31 @@
     }
 
     static Filter toFilter(final Context c, final FilterType type, final String ldapAttribute,
-            final Object valueAssertion) {
-        final String v = String.valueOf(valueAssertion);
+            final ByteString valueAssertion) {
         final Filter filter;
         switch (type) {
         case CONTAINS:
-            filter = Filter.substrings(ldapAttribute, null, Collections.singleton(v), null);
+            filter =
+                    Filter.substrings(ldapAttribute, null, Collections.singleton(valueAssertion),
+                            null);
             break;
         case STARTS_WITH:
-            filter = Filter.substrings(ldapAttribute, v, null, null);
+            filter = Filter.substrings(ldapAttribute, valueAssertion, null, null);
             break;
         case EQUAL_TO:
-            filter = Filter.equality(ldapAttribute, v);
+            filter = Filter.equality(ldapAttribute, valueAssertion);
             break;
         case GREATER_THAN:
-            filter = Filter.greaterThan(ldapAttribute, v);
+            filter = Filter.greaterThan(ldapAttribute, valueAssertion);
             break;
         case GREATER_THAN_OR_EQUAL_TO:
-            filter = Filter.greaterOrEqual(ldapAttribute, v);
+            filter = Filter.greaterOrEqual(ldapAttribute, valueAssertion);
             break;
         case LESS_THAN:
-            filter = Filter.lessThan(ldapAttribute, v);
+            filter = Filter.lessThan(ldapAttribute, valueAssertion);
             break;
         case LESS_THAN_OR_EQUAL_TO:
-            filter = Filter.lessOrEqual(ldapAttribute, v);
+            filter = Filter.lessOrEqual(ldapAttribute, valueAssertion);
             break;
         case PRESENT:
             filter = Filter.present(ldapAttribute);
@@ -309,6 +339,25 @@
         return s != null ? s.toLowerCase(Locale.ENGLISH) : null;
     }
 
+    /**
+     * Returns a result handler which accepts results of type {@code M}, applies
+     * the function {@code f} in order to convert the result to an object of
+     * type {@code N}, and subsequently invokes {@code handler}. If an
+     * unexpected error occurs while performing the transformation, the
+     * exception is converted to a {@code ResourceException} before invoking
+     * {@code handler.handleError()}.
+     *
+     * @param <M>
+     *            The type of result expected by the returned handler.
+     * @param <N>
+     *            The type of result expected by {@code handler}.
+     * @param f
+     *            A function which converts the result of type {@code M} to type
+     *            {@code N}.
+     * @param handler
+     *            A result handler which accepts results of type {@code N}.
+     * @return A result handler which accepts results of type {@code M}.
+     */
     static <M, N> ResultHandler<M> transform(final Function<M, N, Void> f,
             final ResultHandler<N> handler) {
         return new ResultHandler<M>() {
@@ -319,7 +368,11 @@
 
             @Override
             public void handleResult(final M result) {
-                handler.handleResult(f.apply(result, null));
+                try {
+                    handler.handleResult(f.apply(result, null));
+                } catch (Throwable t) {
+                    handler.handleError(adapt(t));
+                }
             }
         };
     }

--
Gitblit v1.10.0