From 5209c98aac8c837510ebd1f51eb0744b4526a605 Mon Sep 17 00:00:00 2001 From: Sven Helmberger <fforw@gmx.de> Date: Sat, 7 Nov 2015 13:19:25 +0100 Subject: [PATCH] Improve JSONifying behavior of cyclic structures. One of the things svenson does not do for you is to take care of cyclic structure. Instead it expects you to make sure that you @JSONPropery(ignore=true) the right back references to solve the issue. It would just fail with a StackoverflowError when you fail to do so or when you use types that are cyclic by nature. See also https://github.com/fforw/svenson/issues/1 (not actually the first issue, just the first one since we moved to github) One reason why the StackOverflowError is so annoying lies in the its very nature of being a stack overflow and its stack trace being a loooong chain of repetitive method invocations. Thankfully there is one easy fix we can do right now without deciding on the more complicated issues outline in the bug. We already do the JSONifying in a private recursive method which is called from public non-recursive methods. Which means that if we catch the stack overflow error there, we can wrap it in another exception whose stack trace then shows us the entry point into the JSONification process, greatly aiding debugging. --- .../org/svenson/CyclicStructureException.java | 25 ++++++++++++ src/main/java/org/svenson/JSON.java | 18 ++++++++- .../org/svenson/CyclicStructureTestCase.java | 39 +++++++++++++++++++ .../java/org/svenson/test/CyclicBean.java | 36 +++++++++++++++++ 4 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 src/main/java/org/svenson/CyclicStructureException.java create mode 100644 src/test/java/org/svenson/CyclicStructureTestCase.java create mode 100644 src/test/java/org/svenson/test/CyclicBean.java diff --git a/src/main/java/org/svenson/CyclicStructureException.java b/src/main/java/org/svenson/CyclicStructureException.java new file mode 100644 index 0000000..7f98556 --- /dev/null +++ b/src/main/java/org/svenson/CyclicStructureException.java @@ -0,0 +1,25 @@ +package org.svenson; + +public class CyclicStructureException + extends SvensonRuntimeException +{ + private static final long serialVersionUID = -4937201415177884428L; + + + public CyclicStructureException(String message, Throwable cause) + { + super(message, cause); + } + + + public CyclicStructureException(String message) + { + super(message); + } + + + public CyclicStructureException(Throwable cause) + { + super(cause); + } +} diff --git a/src/main/java/org/svenson/JSON.java b/src/main/java/org/svenson/JSON.java index b22e81a..9f31356 100644 --- a/src/main/java/org/svenson/JSON.java +++ b/src/main/java/org/svenson/JSON.java @@ -249,7 +249,14 @@ private static void newLine(StringBuilder sb, int cnt) */ public void dumpObject(JSONCharacterSink out, Object o) { - dumpObject(out, o, '\0', ignoredProperties); + try + { + dumpObject(out, o, '\0', ignoredProperties); + } + catch(StackOverflowError e) + { + throw new CyclicStructureException("Cyclic JSON structure", e); + } } /** @@ -263,7 +270,14 @@ public void dumpObject(JSONCharacterSink out, Object o) public void dumpObject(JSONCharacterSink out, Object o, Collection<String> ignoredProps) { - dumpObject(out, o, '\0', ignoredProps); + try + { + dumpObject(out, o, '\0', ignoredProps); + } + catch(StackOverflowError e) + { + throw new CyclicStructureException("Cyclic JSON structure", e); + } } /** diff --git a/src/test/java/org/svenson/CyclicStructureTestCase.java b/src/test/java/org/svenson/CyclicStructureTestCase.java new file mode 100644 index 0000000..4e7ef93 --- /dev/null +++ b/src/test/java/org/svenson/CyclicStructureTestCase.java @@ -0,0 +1,39 @@ +package org.svenson; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.notification.Failure; +import org.svenson.test.CyclicBean; + +import java.io.PrintWriter; +import java.io.StringWriter; + +import static org.hamcrest.Matchers.*; +import static org.hamcrest.MatcherAssert.*; + +public class CyclicStructureTestCase +{ + @Test + public void testCycle() throws Exception + { + + CyclicBean bean = new CyclicBean(); + bean.setInner(new CyclicBean.Inner(bean)); + + try + { + JSON.defaultJSON().forValue(bean); + + } + catch(CyclicStructureException e) + { + StringWriter sw = new StringWriter(); + e.printStackTrace(new PrintWriter(sw)); + + // make sure the stacktrace shows this method + assertThat(sw.getBuffer().toString(),containsString("org.svenson.CyclicStructureTestCase.testCycle")); + return; + } + Assert.fail("Should have thrown a CyclicStructureException"); + } +} diff --git a/src/test/java/org/svenson/test/CyclicBean.java b/src/test/java/org/svenson/test/CyclicBean.java new file mode 100644 index 0000000..7569a34 --- /dev/null +++ b/src/test/java/org/svenson/test/CyclicBean.java @@ -0,0 +1,36 @@ +package org.svenson.test; + +public class CyclicBean +{ + private Inner inner; + + + public void setInner(Inner inner) + { + this.inner = inner; + } + + + public Inner getInner() + { + return inner; + } + + + public static class Inner + { + private final CyclicBean cyclicBean; + + + public Inner(CyclicBean cyclicBean) + { + this.cyclicBean = cyclicBean; + } + + + public CyclicBean getCyclicBean() + { + return cyclicBean; + } + } +}