feat(jans-orm): allow to persist JSONObject by yurem · Pull Request #13939 · JanssenProject/jans (original) (raw)

1634-1652: 🧹 Nitpick | 🔵 Trivial

Minor: use the existing JSONObject import.

org.json.JSONObject is already imported at line 33 and referenced by its short name elsewhere in this file (e.g. lines 1260, 1759). Using the FQN here is inconsistent. Same applies to line 2107–2108.

♻️ Proposed cleanup

```
       String value;
```

       if (propertyValue instanceof org.json.JSONObject) {

           value = ((org.json.JSONObject) propertyValue).toString();

```
       } else {
```

           value = JSON_OBJECT_MAPPER.writeValueAsString(propertyValue);

```
       }
```

```
       String value;
```

       if (propertyValue instanceof JSONObject) {

           value = ((JSONObject) propertyValue).toString();

```
       } else {
```

           value = JSON_OBJECT_MAPPER.writeValueAsString(propertyValue);

```
       }
```

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@jans-orm/core/src/main/java/io/jans/orm/impl/BaseEntryManager.java` around
lines 1634 - 1652, The code in BaseEntryManager.convertValueToJson is using the
fully-qualified org.json.JSONObject instead of the existing short import; update
the reference to use the imported JSONObject type (i.e., replace
org.json.JSONObject with JSONObject) and make the same consistency fix for the
other occurrence(s) that use the FQN in this class (around the JSON conversion
methods) so all JSONObject usages rely on the single import; keep the method
behavior and exception handling unchanged.

2104-2118: ⚠️ Potential issue | 🟡 Minor

Asymmetric type check + null-value regression in convertJsonToValue.

Two concerns with the deserialization branch:

equals vs instanceof asymmetry. convertValueToJson (line 1641) uses instanceof, which also matches subclasses of JSONObject, but here you use parameterType.equals(org.json.JSONObject.class), which requires an exact class match. If a property is declared with a JSONObject subclass (or the user passes one via Class<?>), serialization will use the native path while deserialization falls through to Jackson — which will not reconstruct a JSONObject correctly, breaking round-trip. Prefer org.json.JSONObject.class.isAssignableFrom(parameterType) for consistency with the write path.
Null propertyValue now throws. When propertyValue is null, String.valueOf(propertyValue) produces the literal "null", and new JSONObject("null") throws JSONException. It is caught and re-thrown as MappingException, which is a behavioral change for callers where a null attribute value was previously tolerated via Jackson. A short-circuit for null keeps the old semantics. 🔧 Proposed fix

protected Object convertJsonToValue(Class<?> parameterType, Object propertyValue) {

if (propertyValue == null) {
```
   return null;
```
} try { Object jsonValue;

   if (parameterType.equals(org.json.JSONObject.class)) {

   if (JSONObject.class.isAssignableFrom(parameterType)) {
       jsonValue = new org.json.JSONObject(String.valueOf(propertyValue));
   } else {
       jsonValue = JSON_OBJECT_MAPPER.readValue(String.valueOf(propertyValue), parameterType);
   }
   return jsonValue;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@jans-orm/core/src/main/java/io/jans/orm/impl/BaseEntryManager.java` around
lines 2104 - 2118, The deserialization in convertJsonToValue is asymmetric and
regresses null handling: replace the exact-class check
parameterType.equals(org.json.JSONObject.class) with an assignability check
(org.json.JSONObject.class.isAssignableFrom(parameterType)) to match
convertValueToJson behavior, and short-circuit when propertyValue is null
(return null) before calling String.valueOf(...) or new
org.json.JSONObject(...); keep JSON_OBJECT_MAPPER.readValue(...) and the
existing exception handling/MappingException usage and log via LOG.error as
before.