mirror of https://github.com/OpenIdentityPlatform/OpenDJ.git

Jean-Noël Rouvignac
03.46.2016 a5809829e89ea42665396a628ea277d253ef9fa1
Do not put JsonValue inside JSON schemas

With COMMONS-133, OpenDJ is not able to start itself.

Running OpenDJ with the current PR applied results in the following exception:
```
(org.forgerock.opendj.rest2ldap.Rest2LdapJsonConfiguratorTest) Time elapsed: 0.085 sec <<< FAILURE!
java.lang.IllegalStateException: The given Schema name 'frapi:opendj:rest2ldap:object:1.0' already exists but the Schema objects are not equal
at org.forgerock.api.CrestApiProducer.merge(CrestApiProducer.java:129)
at org.forgerock.api.CrestApiProducer.merge(CrestApiProducer.java:45)
at org.forgerock.services.routing.AbstractRouter.buildApi(AbstractRouter.java:369)
at org.forgerock.services.routing.AbstractRouter.notifyDescriptorChange(AbstractRouter.java:342)
at org.forgerock.services.routing.AbstractRouter.api(AbstractRouter.java:290)
at org.forgerock.opendj.rest2ldap.Resource.subResourcesApi(Resource.java:652)
at org.forgerock.opendj.rest2ldap.SubResource$SubResourceHandler.api(SubResource.java:221)
at org.forgerock.opendj.rest2ldap.SubResource$SubResourceHandler.api(SubResource.java:141)
at org.forgerock.services.routing.AbstractRouter.buildApi(AbstractRouter.java:361)
at org.forgerock.services.routing.AbstractRouter.notifyDescriptorChange(AbstractRouter.java:342)
at org.forgerock.services.routing.AbstractRouter.api(AbstractRouter.java:290)
at org.forgerock.opendj.rest2ldap.DescribableRequestHandler.api(DescribableRequestHandler.java:109)
at org.forgerock.opendj.rest2ldap.DescribableRequestHandler.api(DescribableRequestHandler.java:40)
at org.forgerock.services.routing.AbstractRouter.buildApi(AbstractRouter.java:361)
at org.forgerock.services.routing.AbstractRouter.notifyDescriptorChange(AbstractRouter.java:342)
at org.forgerock.services.routing.AbstractRouter.api(AbstractRouter.java:290)
at org.forgerock.json.resource.FilterChain.api(FilterChain.java:272)
at org.forgerock.json.resource.FilterChain.api(FilterChain.java:38)
at org.forgerock.services.routing.AbstractRouter.buildApi(AbstractRouter.java:361)
at org.forgerock.services.routing.AbstractRouter.notifyDescriptorChange(AbstractRouter.java:342)
at org.forgerock.services.routing.AbstractRouter.api(AbstractRouter.java:290)
at org.forgerock.opendj.rest2ldap.DescribableRequestHandler.api(DescribableRequestHandler.java:109)
at org.forgerock.opendj.rest2ldap.Rest2LdapJsonConfiguratorTest.configureEndpoints(Rest2LdapJsonConfiguratorTest.java:81)
at org.forgerock.opendj.rest2ldap.Rest2LdapJsonConfiguratorTest.testConfigureEndpointsWithApiDescription(Rest2LdapJsonConfiguratorTest.java:61)
```

The problem arises in `CrestApiProducer.merge(List<ApiDescription> descriptions)` when it calls `Definitions.put(String, Schema)`.
Here are the lines throwing the exception:
```java
if (definitions.containsKey(name) && !definitions.get(name).equals(schema)) {
throw new IllegalStateException("The given Schema name"
+ " '" + name + "' already exists but the Schema objects are not equal");
}
```
In our case, we are generating the whole type hierarchy for the definition in use for each ApiDescription.
We do that because we cannot possibly know if a type has already been seen before or not.

In this case, this happens when trying to merge definitions for `frapi:opendj:rest2ldap:object:1.0`
which is our base type (think about `java.lang.Object`). They have the same textual json schema.

This happens because `JsonValue.equals(JsonValue)` is never equal.
This is caused by a lack of unwrapping in `JsonValue.put(String, Object)` and other such methods.
It may be fixed by COMMONS-135.

The fix in OpenDJ is to always unwrap the JsonValue before calling `JsonValue.put(String, Object)`.
6 files modified
29 ■■■■■ changed files
opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/JsonConstantPropertyMapper.java 7 ●●●●● patch | view | raw | blame | history
opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ObjectPropertyMapper.java 4 ●●●● patch | view | raw | blame | history
opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/PropertyMapper.java 8 ●●●● patch | view | raw | blame | history
opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ReferencePropertyMapper.java 2 ●●● patch | view | raw | blame | history
opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Resource.java 6 ●●●● patch | view | raw | blame | history
opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/SimplePropertyMapper.java 2 ●●● patch | view | raw | blame | history
opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/JsonConstantPropertyMapper.java
@@ -171,13 +171,14 @@
            for (String key : value.keys()) {
                jsonProps.put(key, toJsonSchema(value.get(key)));
            }
            jsonSchema.put("properties", jsonSchema);
            jsonSchema.put("properties", jsonProps.getObject());
            return jsonSchema;
        } else if (value.isCollection()) {
            final JsonValue jsonSchema = json(object(field("type", "array")));
            final JsonValue firstItem = value.get(value.keys().iterator().next());
            // assume all items have the same schema
            jsonSchema.put("items", toJsonSchema(firstItem));
            JsonValue firstItemJson = toJsonSchema(firstItem);
            jsonSchema.put("items", firstItemJson != null ? firstItemJson.getObject() : null);
            if (value.getObject() instanceof Set) {
                jsonSchema.put("uniqueItems", true);
            }
@@ -194,7 +195,7 @@
        } else if (value.isNull()) {
            return json(object(field("type", "null")));
        } else {
            return null;
            throw new IllegalStateException("Unsupported json value: " + value);
        }
    }
}
opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ObjectPropertyMapper.java
@@ -446,7 +446,7 @@
        for (Mapping mapping : mappings.values()) {
            final String attribute = mapping.name;
            PropertyMapper mapper = mapping.mapper;
            jsonProps.put(attribute, mapper.toJsonSchema());
            jsonProps.put(attribute, mapper.toJsonSchema().getObject());
            if (mapper.isRequired()) {
                requiredFields.add(attribute);
            }
@@ -457,7 +457,7 @@
            jsonSchema.put("required", requiredFields);
        }
        if (jsonProps.size() > 0) {
            jsonSchema.put("properties", jsonProps);
            jsonSchema.put("properties", jsonProps.getObject());
        }
        return jsonSchema;
    }
opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/PropertyMapper.java
@@ -186,6 +186,8 @@
     *
     * @param context The request context.
     * @param resource The exact type of resource being updated.
     * @param path the path to update.
     * @param e the entry containing the new value.
     * @param v
     *            The JSON value to be converted to LDAP attributes, which may
     *            be {@code null} indicating that the JSON value was not present
@@ -195,8 +197,12 @@
    abstract Promise<List<Modification>, ResourceException> update(Context context, Resource resource,
                                                                   JsonPointer path, Entry e, JsonValue v);
    // TODO: methods for obtaining schema information (e.g. name, description, type information).
    // TODO: methods for creating sort controls.
    /**
     * Returns the non-null JSON schema for this property mapper.
     *
     * @return the non-null JSON schema for this property mapper
     */
    abstract JsonValue toJsonSchema();
}
opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ReferencePropertyMapper.java
@@ -363,7 +363,7 @@
        if (mapper.isMultiValued()) {
            final JsonValue jsonSchema = json(object(
                field("type", "array"),
                field("items", mapper.toJsonSchema()),
                field("items", mapper.toJsonSchema().getObject()),
                // LDAP has set semantics => all items are unique
                field("uniqueItems", true)));
            putWritabilityProperties(jsonSchema);
opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Resource.java
@@ -857,7 +857,7 @@
            }
            final JsonValue jsonSchema = mapper.toJsonSchema();
            if (jsonSchema != null) {
                properties.put(propertyName, jsonSchema);
                properties.put(propertyName, jsonSchema.getObject());
            }
        }
@@ -870,14 +870,14 @@
            jsonSchema.put("required", requiredFields);
        }
        if (properties.size() > 0) {
            jsonSchema.put("properties", properties);
            jsonSchema.put("properties", properties.getObject());
        }
        if (superType != null) {
            return schema(json(object(
                field("allOf", array(
                    object(field("$ref", "#/definitions/" + superType.id)),
                    jsonSchema)))));
                    jsonSchema.getObject())))));
        }
        return schema(jsonSchema);
    }
opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/SimplePropertyMapper.java
@@ -228,7 +228,7 @@
                field("type", "array"),
                // LDAP has set semantics => all items are unique
                field("uniqueItems", true),
                field("items", itemsSchema(attrType))));
                field("items", itemsSchema(attrType).getObject())));
        } else {
            jsonSchema = itemsSchema(attrType);
        }