Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved JSON.stringify for wrapped java objects #824

Closed
wants to merge 7 commits into from
Closed

Improved JSON.stringify for wrapped java objects #824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f1f2c58
Improved JSON.stringify for wrapped java objects
rPraml Jan 14, 2021
0d9adf6
Merge remote-tracking branch 'upstream/master' into son-stringify-imp…
rPraml Apr 20, 2021
ed6e7d1
update PR
rPraml Apr 20, 2021
3304508
Update :spotlessApply
rPraml Apr 20, 2021
66f4f45
Updated config.yml
rPraml Apr 20, 2021
8e59bf9
ADD: NativeJavaObject.toJSON
rPraml Apr 20, 2021
c4e2033
Merge branch 'son-stringify-improved' of github.com:FOCONIS/rhino int…
rPraml Apr 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/org/mozilla/javascript/ArrayScriptable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/* -*- Mode: java; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

package org.mozilla.javascript;

/**
* Common interface for {@link NativeArray}, {@link NativeJavaArray} and {@link NativeJavaList}.
* Required for JSON conversion.
*
* @author Roland Praml, FOCONIS AG
*/
public interface ArrayScriptable extends Scriptable {

/** Returns the length of the array. */
long getLength();
}
3 changes: 1 addition & 2 deletions src/org/mozilla/javascript/NativeArray.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@
* @author Norris Boyd
* @author Mike McCabe
*/
public class NativeArray extends IdScriptableObject implements List
{
public class NativeArray extends IdScriptableObject implements List, ArrayScriptable {
private static final long serialVersionUID = 7331366857676127338L;

/*
Expand Down
151 changes: 107 additions & 44 deletions src/org/mozilla/javascript/NativeJSON.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,17 @@

package org.mozilla.javascript;

import java.time.temporal.Temporal;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Collection;
import java.util.Date;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Stack;
import java.util.UUID;

import org.mozilla.javascript.json.JsonParser;

Expand Down Expand Up @@ -54,9 +58,10 @@ protected void initPrototypeId(int id)
String name;
int arity;
switch (id) {
case Id_toSource: arity = 0; name = "toSource"; break;
case Id_parse: arity = 2; name = "parse"; break;
case Id_stringify: arity = 3; name = "stringify"; break;
case Id_toSource: arity = 0; name = "toSource"; break;
case Id_parse: arity = 2; name = "parse"; break;
case Id_stringify: arity = 3; name = "stringify"; break;
case Id_javaConverter: arity = 1; name = "javaConverter"; break;
default: throw new IllegalStateException(String.valueOf(id));
}
initPrototypeMethod(JSON_TAG, id, name, arity);
Expand Down Expand Up @@ -102,7 +107,29 @@ public Object execIdCall(IdFunctionObject f, Context cx, Scriptable scope,
}
}

return stringify(cx, scope, value, replacer, space);
return stringify(cx, scope, thisObj, value, replacer, space);
}

case Id_javaConverter: {
if (args.length > 0) {
Object value = args[0];
if (value instanceof java.sql.Date) { // Date-Time handling.
value = ((java.sql.Date) value).toLocalDate();
} else if (value instanceof java.sql.Time) {
value = ((java.sql.Time) value).toLocalTime();
} else if (value instanceof java.util.Date) {
value = ((Date) value).toInstant();
} else if (value instanceof Calendar) {
value = ((Calendar)value).toInstant();
}
if (value instanceof Temporal
|| value instanceof UUID) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHECKME: There are a lot of candidates, that may be converted with "toString". Locale, Duration, ...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how you did this. I was thinking almost the same thing. I still think the default should be to throw TypeErrors, but it should be easy to swap in this implementation. What do you think about another property on the JSON object that is a behavior selector for java objects between TypeError, javaConverter, empty object, or undefined, with this default implementation of javaConverter being provided?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is, to distinguish, when use "toString" and when throw a TypeError (or return undefined):
Some brainstorming:
Idea 1: IF obj.getClass().startsWith("java.") THEN return obj.toString ELSE throw TypeError
This will cover all java objects from the JRE. But there may be some custom add ons (e.g. joda-time) where toString makes also sense

Idea 2: IF isLiveConnect(obj) THEN return obj.toString ELSE throw TypeError
The idea is, to detect, if the object comes from outside (not from the org.mozilla.javascript package). In this case, toString may be the best choice.

Idea 3: The user has to provide an implementation to convert LiveConnect objects to JSON.
This is the idea, I mostly like.
We use a lot of Java-Beans and use Jackson on the java side. It would be great, if we produce the same result like jackson.
And the javaConverter already goes in that direction, but I'm not yet sure if this is the best solution for now.

We use a shared scope for several threads. So if one thread would modify the javaConverter (which should not be possible, as NativeJSON should be sealed), the other thread would see the modification.

What would you think, when we delegate all unconvertable objects to the WrapFactory and it can decide, what to do.

Object json = cx.getWrapFactory().javaToJson(value, indent, gap)
if (json != null) {
    return json;
}
if (!Undefined.isUndefined(value)) {
    throw ScriptRuntime.typeErrorById("msg.json.cant.serialize", value.getClass().getName());
}

return value.toString();
} else if (value instanceof Enum) {
return ((Enum) value).name();
}
}
return Undefined.instance;
}

default: throw new IllegalStateException(String.valueOf(methodId));
Expand Down Expand Up @@ -189,30 +216,32 @@ private static String repeat(char c, int count) {
}

private static class StringifyState {
StringifyState(Context cx, Scriptable scope, String indent, String gap,
Callable replacer, List<Object> propertyList)
StringifyState(Context cx, Scriptable scope, Scriptable thisObj, String indent,
String gap, Callable replacer, List<Object> propertyList)
{
this.cx = cx;
this.scope = scope;
this.thisObj = thisObj;

this.indent = indent;
this.gap = gap;
this.replacer = replacer;
this.propertyList = propertyList;
}

Stack<Scriptable> stack = new Stack<Scriptable>();
Stack<Object> stack = new Stack<Object>();
String indent;
String gap;
Callable replacer;
List<Object> propertyList;

Context cx;
Scriptable scope;
Scriptable thisObj;
}

public static Object stringify(Context cx, Scriptable scope, Object value,
Object replacer, Object space)
public static Object stringify(Context cx, Scriptable scope, Scriptable thisObj,
Object value, Object replacer, Object space)
{
String indent = "";
String gap = "";
Expand Down Expand Up @@ -252,7 +281,7 @@ public static Object stringify(Context cx, Scriptable scope, Object value,
}
}

StringifyState state = new StringifyState(cx, scope,
StringifyState state = new StringifyState(cx, scope, thisObj,
indent,
gap,
replacerFunction,
Expand Down Expand Up @@ -287,35 +316,24 @@ private static Object str(Object key, Scriptable holder,
value = state.replacer.call(state.cx, state.scope, holder,
new Object[] { key, value });
}

return strVal(value, holder, state);
}

private static Object strVal(Object value, Scriptable holder,
StringifyState state) {

if (value instanceof NativeNumber) {
value = Double.valueOf(ScriptRuntime.toNumber(value));
} else if (value instanceof NativeString) {
value = ScriptRuntime.toString(value);
} else if (value instanceof NativeBoolean) {
value = ((NativeBoolean) value).getDefaultValue(ScriptRuntime.BooleanClass);
} else if (value instanceof NativeJavaMap
|| value instanceof ArrayScriptable) {
// do not unpack native maps && lists
} else if (value instanceof NativeJavaObject) {
value = ((NativeJavaObject) value).unwrap();
if (value instanceof Map) {
Map<?,?> map = (Map<?,?>) value;
NativeObject nObj = new NativeObject();
map.forEach((k, v) -> {
if (k instanceof CharSequence) {
nObj.put(((CharSequence) k).toString(), nObj, v);
}
});
value = nObj;
}
else {
if (value instanceof Collection<?>) {
Collection<?> col = (Collection<?>) value;
value = col.toArray(new Object[col.size()]);
}
if (value instanceof Object[]) {
value = new NativeArray((Object[]) value);
}
}
}
}

if (value == null) return "null";
if (value.equals(Boolean.TRUE)) return "true";
Expand All @@ -334,15 +352,48 @@ private static Object str(Object key, Scriptable holder,
}
return "null";
}

// pack maps & lists
if (value instanceof Map) {
Map<?, ?> map = (Map<?, ?>) value;
NativeObject nObj = new NativeObject();
map.forEach((k, v) -> {
if (k instanceof CharSequence) {
nObj.put(k.toString(), nObj, v);
}
});
value = nObj;
} else if (value instanceof Collection<?>) {
Collection<?> col = (Collection<?>) value;
value = col.toArray(new Object[col.size()]);
} else if (value instanceof Object[]) {
value = new NativeArray((Object[]) value);
}

if (value instanceof Scriptable) {
if (!(value instanceof Callable)) {
if (value instanceof NativeArray) {
return ja((NativeArray) value, state);
if (value instanceof ArrayScriptable) {
return ja((ArrayScriptable) value, state);
}
return jo((Scriptable) value, state);
}
} else if (!Undefined.isUndefined(value)) {
}
String javaConverter = "javaConverter";
Object converter = ScriptableObject.getProperty(state.thisObj, javaConverter);
if (converter != NOT_FOUND) {
if ( !(converter instanceof Callable) ) {
throw ScriptRuntime.typeErrorById("msg.isnt.function.in",
javaConverter,
ScriptRuntime.toString(state.thisObj),
ScriptRuntime.toString(converter));
}
Object result = ((Callable) converter).call(state.cx, state.scope, state.scope, new Object[]{value});
if (!Undefined.isUndefined(result)) {
return strVal(result, holder,state);
}
}

if (!Undefined.isUndefined(value)) {
throw ScriptRuntime.typeErrorById("msg.json.cant.serialize", value.getClass().getName());
}

Expand All @@ -363,10 +414,14 @@ private static String join(Collection<Object> objs, String delimiter) {
}

private static String jo(Scriptable value, StringifyState state) {
if (state.stack.search(value) != -1) {
throw ScriptRuntime.typeErrorById("msg.cyclic.value");
Object trackValue = value;
if (value instanceof Wrapper) {
trackValue = ((Wrapper) value).unwrap();
}
if (state.stack.search(trackValue) != -1) {
throw ScriptRuntime.typeErrorById("msg.cyclic.value", trackValue.getClass().getName());
}
state.stack.push(value);
state.stack.push(trackValue);

String stepback = state.indent;
state.indent = state.indent + state.gap;
Expand Down Expand Up @@ -411,11 +466,15 @@ private static String jo(Scriptable value, StringifyState state) {
return finalValue;
}

private static String ja(NativeArray value, StringifyState state) {
if (state.stack.search(value) != -1) {
throw ScriptRuntime.typeErrorById("msg.cyclic.value");
private static String ja(ArrayScriptable value, StringifyState state) {
Object trackValue = value;
if (value instanceof Wrapper) {
trackValue = ((Wrapper) value).unwrap();
}
state.stack.push(value);
if (state.stack.search(trackValue) != -1) {
throw ScriptRuntime.typeErrorById("msg.cyclic.value", trackValue.getClass().getName());
}
state.stack.push(trackValue);

String stepback = state.indent;
state.indent = state.indent + state.gap;
Expand Down Expand Up @@ -505,7 +564,7 @@ private static String quote(String string) {
protected int findPrototypeId(String s)
{
int id;
// #generated# Last update: 2021-03-21 09:51:17 MEZ
// #generated# Last update: 2021-04-20 15:02:52 MESZ
switch (s) {
case "toSource":
id = Id_toSource;
Expand All @@ -516,6 +575,9 @@ protected int findPrototypeId(String s)
case "stringify":
id = Id_stringify;
break;
case "javaConverter":
id = Id_javaConverter;
break;
default:
id = 0;
break;
Expand All @@ -528,8 +590,9 @@ protected int findPrototypeId(String s)
Id_toSource = 1,
Id_parse = 2,
Id_stringify = 3,
LAST_METHOD_ID = 3,
MAX_ID = 3;
Id_javaConverter= 4,
LAST_METHOD_ID = 4,
MAX_ID = 4;

// #/string_id_map#
}
7 changes: 6 additions & 1 deletion src/org/mozilla/javascript/NativeJavaArray.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

public class NativeJavaArray
extends NativeJavaObject
implements SymbolScriptable
implements SymbolScriptable, ArrayScriptable
{
private static final long serialVersionUID = -924022554283675333L;

Expand Down Expand Up @@ -139,6 +139,11 @@ public Object[] getIds() {
result[i] = Integer.valueOf(i);
return result;
}

@Override
public long getLength() {
return length;
}

@Override
public boolean hasInstance(Scriptable value) {
Expand Down
9 changes: 7 additions & 2 deletions src/org/mozilla/javascript/NativeJavaList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import java.util.List;

public class NativeJavaList extends NativeJavaObject {
public class NativeJavaList extends NativeJavaObject implements ArrayScriptable {

private List<Object> list;

Expand Down Expand Up @@ -61,7 +61,7 @@ public Object get(int index, Scriptable start) {
if (isWithValidIndex(index)) {
Context cx = Context.getCurrentContext();
Object obj = list.get(index);
if (cx != null) {
if (cx != null && obj != null) {
return cx.getWrapFactory().wrap(cx, this, obj, obj.getClass());
}
return obj;
Expand All @@ -86,6 +86,11 @@ public void put(int index, Scriptable start, Object value) {
super.put(index, start, value);
}

@Override
public long getLength() {
return list.size();
}

@Override
public Object[] getIds() {
List<?> list = (List<?>) javaObject;
Expand Down
6 changes: 6 additions & 0 deletions src/org/mozilla/javascript/NativeJavaMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ public Object get(String name, Scriptable start) {
if (cx != null && cx.hasFeature(Context.FEATURE_ENABLE_JAVA_MAP_ACCESS)) {
if (map.containsKey(name)) {
Object obj = map.get(name);
if (obj == null) {
return null;
}
return cx.getWrapFactory().wrap(cx, this, obj, obj.getClass());
}
}
Expand All @@ -78,6 +81,9 @@ public Object get(int index, Scriptable start) {
if (cx != null && cx.hasFeature(Context.FEATURE_ENABLE_JAVA_MAP_ACCESS)) {
if (map.containsKey(Integer.valueOf(index))) {
Object obj = map.get(Integer.valueOf(index));
if (obj == null) {
return null;
}
return cx.getWrapFactory().wrap(cx, this, obj, obj.getClass());
}
}
Expand Down
Loading