From 53dd810b9aed6877dd1ef17ed83d3991f5aa33d8 Mon Sep 17 00:00:00 2001 From: Robin Bygrave Date: Thu, 3 Mar 2016 23:38:09 +1300 Subject: [PATCH] #587 - Refactor - Change JsonContext.getScalar() to JsonContext.writeScalar() --- .../avaje/ebean/text/json/JsonContext.java | 5 +-- .../com/avaje/ebean/text/json/JsonScalar.java | 15 --------- .../server/changelog/ChangeJsonBuilder.java | 12 +++---- .../server/el/ElPropertyChain.java | 1 - .../expression/ElasticExpressionContext.java | 32 +++++++++++-------- .../server/querydefn/DefaultOrmQuery.java | 8 ++--- .../server/text/json/DJsonContext.java | 8 +++-- ...efaultJsonScalar.java => DJsonScalar.java} | 17 ++++------ .../server/expression/BaseElasticTest.java | 11 ++----- .../SimpleExpressionElasticTest.java | 9 ++---- .../querydefn/DefaultOrmQueryElasticTest.java | 14 +++----- 11 files changed, 48 insertions(+), 84 deletions(-) delete mode 100644 src/main/java/com/avaje/ebean/text/json/JsonScalar.java rename src/main/java/com/avaje/ebeaninternal/server/text/json/{DefaultJsonScalar.java => DJsonScalar.java} (60%) diff --git a/src/main/java/com/avaje/ebean/text/json/JsonContext.java b/src/main/java/com/avaje/ebean/text/json/JsonContext.java index adcd55ba5..8f5d424d6 100644 --- a/src/main/java/com/avaje/ebean/text/json/JsonContext.java +++ b/src/main/java/com/avaje/ebean/text/json/JsonContext.java @@ -4,6 +4,7 @@ import com.avaje.ebean.FetchPath; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonParser; +import java.io.IOException; import java.io.Reader; import java.io.Writer; import java.lang.reflect.Type; @@ -213,13 +214,13 @@ public interface JsonContext { JsonParser createParser(Reader reader) throws JsonIOException; /** - * Return a helper that can write scalar types known to Ebean to Jackson. + * Write a scalar types known to Ebean to Jackson. *

* Ebean has built in support for java8 and Joda types as well as the other * standard JDK types like URI, URL, UUID etc. This is a fast simple way to * write any of those types to Jackson. *

*/ - JsonScalar getScalar(JsonGenerator generator); + void writeScalar(JsonGenerator generator, Object scalarValue) throws IOException; } \ No newline at end of file diff --git a/src/main/java/com/avaje/ebean/text/json/JsonScalar.java b/src/main/java/com/avaje/ebean/text/json/JsonScalar.java deleted file mode 100644 index 2dc70dc55..000000000 --- a/src/main/java/com/avaje/ebean/text/json/JsonScalar.java +++ /dev/null @@ -1,15 +0,0 @@ -package com.avaje.ebean.text.json; - -import java.io.IOException; - -/** - * Writes any scalar type known to Ebean to the underlying Jackson generator. - */ -public interface JsonScalar { - - /** - * Write the scalar type to JSON where the value can be any type known to Ebean - * including Enums, Java8 time types, Joda types, URL, URI etc. - */ - void write(Object value) throws IOException; -} diff --git a/src/main/java/com/avaje/ebeaninternal/server/changelog/ChangeJsonBuilder.java b/src/main/java/com/avaje/ebeaninternal/server/changelog/ChangeJsonBuilder.java index b6c092673..507d3ad4f 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/changelog/ChangeJsonBuilder.java +++ b/src/main/java/com/avaje/ebeaninternal/server/changelog/ChangeJsonBuilder.java @@ -5,7 +5,6 @@ import com.avaje.ebean.event.changelog.BeanChange; import com.avaje.ebean.event.changelog.ChangeSet; import com.avaje.ebean.event.changelog.ChangeType; import com.avaje.ebean.text.json.JsonContext; -import com.avaje.ebean.text.json.JsonScalar; import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonGenerator; @@ -97,10 +96,7 @@ public class ChangeJsonBuilder { if (bean.getType() != ChangeType.DELETE) { gen.writeFieldName("values"); gen.writeStartObject(); - // use JsonScalar as it knows how to encode all the scalar - // property types that Ebean supports (Java8, Joda etc) - JsonScalar scalarWriter = json.getScalar(gen); - writeValuePairs(bean, scalarWriter, gen); + writeValuePairs(bean, gen); gen.writeEndObject(); } } @@ -111,7 +107,7 @@ public class ChangeJsonBuilder { * We are intentionally keeping the same new/old structure for both inserts and updates. *

*/ - protected void writeValuePairs(BeanChange bean, JsonScalar scalarWriter, JsonGenerator gen) throws IOException { + protected void writeValuePairs(BeanChange bean, JsonGenerator gen) throws IOException { for (Map.Entry entry : bean.getValues().entrySet()) { gen.writeFieldName(entry.getKey()); @@ -120,12 +116,12 @@ public class ChangeJsonBuilder { Object newValue = value.getNewValue(); if (newValue != null) { gen.writeFieldName("new"); - scalarWriter.write(newValue); + json.writeScalar(gen, newValue); } Object oldValue = value.getOldValue(); if (oldValue != null) { gen.writeFieldName("old"); - scalarWriter.write(oldValue); + json.writeScalar(gen, oldValue); } gen.writeEndObject(); } diff --git a/src/main/java/com/avaje/ebeaninternal/server/el/ElPropertyChain.java b/src/main/java/com/avaje/ebeaninternal/server/el/ElPropertyChain.java index 98fd42a64..9f7214eaa 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/el/ElPropertyChain.java +++ b/src/main/java/com/avaje/ebeaninternal/server/el/ElPropertyChain.java @@ -1,7 +1,6 @@ package com.avaje.ebeaninternal.server.el; import com.avaje.ebean.bean.EntityBean; -import com.avaje.ebean.text.StringFormatter; import com.avaje.ebean.text.StringParser; import com.avaje.ebeaninternal.server.deploy.BeanProperty; import com.avaje.ebeaninternal.server.lib.util.StringHelper; diff --git a/src/main/java/com/avaje/ebeaninternal/server/expression/ElasticExpressionContext.java b/src/main/java/com/avaje/ebeaninternal/server/expression/ElasticExpressionContext.java index 5d5835450..b60a48e9f 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/expression/ElasticExpressionContext.java +++ b/src/main/java/com/avaje/ebeaninternal/server/expression/ElasticExpressionContext.java @@ -3,10 +3,12 @@ package com.avaje.ebeaninternal.server.expression; import com.avaje.ebean.OrderBy; import com.avaje.ebean.plugin.BeanType; import com.avaje.ebean.plugin.ExpressionPath; +import com.avaje.ebean.text.json.JsonContext; import com.avaje.ebeaninternal.server.query.SplitName; import com.fasterxml.jackson.core.JsonGenerator; import java.io.IOException; +import java.io.StringWriter; import java.util.ArrayList; import java.util.List; @@ -30,8 +32,12 @@ public class ElasticExpressionContext { public static final String EXISTS = "exists"; public static final String FIELD = "field"; + private final JsonContext jsonContext; + private final JsonGenerator json; + private final StringWriter writer; + private final BeanType desc; private String currentNestedPath; @@ -39,9 +45,11 @@ public class ElasticExpressionContext { /** * Construct given the JSON generator and root bean type. */ - public ElasticExpressionContext(JsonGenerator json, BeanType desc) { - this.json = json; + public ElasticExpressionContext(JsonContext jsonContext, BeanType desc) { + this.jsonContext = jsonContext; this.desc = desc; + this.writer = new StringWriter(200); + this.json = jsonContext.createGenerator(writer); } /** @@ -54,9 +62,10 @@ public class ElasticExpressionContext { /** * Flush the JsonGenerator buffer. */ - public void flush() throws IOException { + public String flush() throws IOException { endNested(); json.flush(); + return writer.toString(); } /** @@ -77,7 +86,7 @@ public class ElasticExpressionContext { /** * Start Bool MUST or SHOULD. - * + *

* If conjunction is true then MUST(and) and if false is SHOULD(or). */ public void writeBoolStart(boolean conjunction) throws IOException { @@ -137,7 +146,7 @@ public class ElasticExpressionContext { json.writeObjectFieldStart(RANGE); json.writeObjectFieldStart(rawProperty(propertyName)); json.writeFieldName(rangeType); - json.writeObject(value); + jsonContext.writeScalar(json, value); json.writeEndObject(); json.writeEndObject(); json.writeEndObject(); @@ -148,17 +157,14 @@ public class ElasticExpressionContext { */ public void writeRange(String propertyName, Op lowOp, Object valueLow, Op highOp, Object valueHigh) throws IOException { - //Property property = desc.getProperty(propertyName); - //property. - prepareNestedPath(propertyName); json.writeStartObject(); json.writeObjectFieldStart(RANGE); json.writeObjectFieldStart(rawProperty(propertyName)); json.writeFieldName(lowOp.docExp()); - json.writeObject(valueLow); + jsonContext.writeScalar(json, valueLow); json.writeFieldName(highOp.docExp()); - json.writeObject(valueHigh); + jsonContext.writeScalar(json, valueHigh); json.writeEndObject(); json.writeEndObject(); json.writeEndObject(); @@ -174,7 +180,7 @@ public class ElasticExpressionContext { json.writeObjectFieldStart(TERMS); json.writeArrayFieldStart(rawProperty(propertyName)); for (Object value : values) { - json.writeObject(value); + jsonContext.writeScalar(json, value); } json.writeEndArray(); json.writeEndObject(); @@ -191,7 +197,7 @@ public class ElasticExpressionContext { json.writeObjectFieldStart(IDS); json.writeArrayFieldStart(VALUES); for (Object id : idList) { - json.writeObject(id); + jsonContext.writeScalar(json, id); } json.writeEndArray(); json.writeEndObject(); @@ -322,7 +328,7 @@ public class ElasticExpressionContext { json.writeStartObject(); json.writeObjectFieldStart(type); json.writeFieldName(propertyName); - json.writeObject(value); + jsonContext.writeScalar(json, value); json.writeEndObject(); json.writeEndObject(); } diff --git a/src/main/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQuery.java b/src/main/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQuery.java index ba11936f8..5315cadcc 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQuery.java +++ b/src/main/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQuery.java @@ -292,18 +292,14 @@ public class DefaultOrmQuery implements SpiQuery { public String asElasticQuery() { - StringWriter sw = new StringWriter(200); JsonContext json = server.json(); - JsonGenerator generator = json.createGenerator(sw); - BeanType beanType = server.getPluginApi().getBeanType(this.beanType); - ElasticExpressionContext context = new ElasticExpressionContext(generator, beanType); + ElasticExpressionContext context = new ElasticExpressionContext(json, beanDescriptor); try { writeElastic(context); - context.flush(); - generatedSql = sw.toString(); + generatedSql = context.flush(); return generatedSql; } catch (IOException e) { diff --git a/src/main/java/com/avaje/ebeaninternal/server/text/json/DJsonContext.java b/src/main/java/com/avaje/ebeaninternal/server/text/json/DJsonContext.java index 78b88fd1b..693ec76e1 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/text/json/DJsonContext.java +++ b/src/main/java/com/avaje/ebeaninternal/server/text/json/DJsonContext.java @@ -7,7 +7,6 @@ import com.avaje.ebean.text.json.EJson; import com.avaje.ebean.text.json.JsonContext; import com.avaje.ebean.text.json.JsonIOException; import com.avaje.ebean.text.json.JsonReadOptions; -import com.avaje.ebean.text.json.JsonScalar; import com.avaje.ebean.text.json.JsonWriteBeanVisitor; import com.avaje.ebean.text.json.JsonWriteOptions; import com.avaje.ebeaninternal.api.SpiEbeanServer; @@ -51,16 +50,19 @@ public class DJsonContext implements JsonContext { private final JsonConfig.Include defaultInclude; + private final DJsonScalar jsonScalar; + public DJsonContext(SpiEbeanServer server, JsonFactory jsonFactory, TypeManager typeManager) { this.server = server; this.typeManager = typeManager; this.jsonFactory = (jsonFactory != null) ? jsonFactory : new JsonFactory(); this.defaultObjectMapper = this.server.getServerConfig().getObjectMapper(); this.defaultInclude = this.server.getServerConfig().getJsonInclude(); + this.jsonScalar = new DJsonScalar(typeManager); } - public JsonScalar getScalar(JsonGenerator generator) { - return new DefaultJsonScalar(typeManager, new WriteJson(generator, defaultInclude)); + public void writeScalar(JsonGenerator generator, Object scalarValue) throws IOException { + jsonScalar.write(generator, scalarValue); } public boolean isSupportedType(Type genericType) { diff --git a/src/main/java/com/avaje/ebeaninternal/server/text/json/DefaultJsonScalar.java b/src/main/java/com/avaje/ebeaninternal/server/text/json/DJsonScalar.java similarity index 60% rename from src/main/java/com/avaje/ebeaninternal/server/text/json/DefaultJsonScalar.java rename to src/main/java/com/avaje/ebeaninternal/server/text/json/DJsonScalar.java index e91c31709..c55c9be34 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/text/json/DefaultJsonScalar.java +++ b/src/main/java/com/avaje/ebeaninternal/server/text/json/DJsonScalar.java @@ -1,39 +1,34 @@ package com.avaje.ebeaninternal.server.text.json; -import com.avaje.ebean.text.json.JsonScalar; import com.avaje.ebeaninternal.server.type.ScalarType; import com.avaje.ebeaninternal.server.type.TypeManager; +import com.fasterxml.jackson.core.JsonGenerator; import java.io.IOException; /** * Default implementation of JsonScalar. */ -public class DefaultJsonScalar implements JsonScalar { - +public class DJsonScalar { private final TypeManager typeManager; - private final WriteJson writeJson; - - public DefaultJsonScalar(TypeManager typeManager, WriteJson writeJson) { + public DJsonScalar(TypeManager typeManager) { this.typeManager = typeManager; - this.writeJson = writeJson; } - @Override @SuppressWarnings("unchecked") - public void write(Object value) throws IOException { + public void write(JsonGenerator gen, Object value) throws IOException { if (value instanceof String) { - writeJson.writeString((String)value); + gen.writeString((String)value); } else { ScalarType scalarType = typeManager.getScalarType(value.getClass()); if (scalarType == null) { throw new IllegalArgumentException("unhandled type " + value.getClass()); } - scalarType.jsonWrite(writeJson.gen(), value); + scalarType.jsonWrite(gen, value); } } } diff --git a/src/test/java/com/avaje/ebeaninternal/server/expression/BaseElasticTest.java b/src/test/java/com/avaje/ebeaninternal/server/expression/BaseElasticTest.java index 9997aba3e..1fa13b553 100644 --- a/src/test/java/com/avaje/ebeaninternal/server/expression/BaseElasticTest.java +++ b/src/test/java/com/avaje/ebeaninternal/server/expression/BaseElasticTest.java @@ -2,23 +2,18 @@ package com.avaje.ebeaninternal.server.expression; import com.avaje.ebean.BaseTestCase; +import com.avaje.ebean.Ebean; import com.avaje.ebeaninternal.server.deploy.BeanDescriptor; import com.avaje.tests.model.basic.Order; -import com.fasterxml.jackson.core.JsonFactory; -import com.fasterxml.jackson.core.JsonGenerator; import java.io.IOException; -import java.io.StringWriter; public abstract class BaseElasticTest extends BaseTestCase { - public static JsonFactory factory = new JsonFactory(); - - public ElasticExpressionContext context(StringWriter sb) throws IOException { + public ElasticExpressionContext context() throws IOException { BeanDescriptor desc = getBeanDescriptor(Order.class); - JsonGenerator gen = factory.createGenerator(sb); - return new ElasticExpressionContext(gen, desc); + return new ElasticExpressionContext(Ebean.json(), desc); } } \ No newline at end of file diff --git a/src/test/java/com/avaje/ebeaninternal/server/expression/SimpleExpressionElasticTest.java b/src/test/java/com/avaje/ebeaninternal/server/expression/SimpleExpressionElasticTest.java index 2ad96cc4d..df02bf88f 100644 --- a/src/test/java/com/avaje/ebeaninternal/server/expression/SimpleExpressionElasticTest.java +++ b/src/test/java/com/avaje/ebeaninternal/server/expression/SimpleExpressionElasticTest.java @@ -3,24 +3,19 @@ package com.avaje.ebeaninternal.server.expression; import org.junit.Test; -import java.io.StringWriter; - import static org.assertj.core.api.Assertions.assertThat; public class SimpleExpressionElasticTest extends BaseElasticTest { - @Test public void writeElastic() throws Exception { SimpleExpression eqExp = new SimpleExpression("name", Op.EQ, "rob"); - StringWriter sb = new StringWriter(); - ElasticExpressionContext context = context(sb); + ElasticExpressionContext context = context(); eqExp.writeElastic(context); - context.json().flush(); - String json = sb.toString(); + String json = context.flush(); assertThat(json).isEqualTo("{\"term\":{\"name\":\"rob\"}}"); } diff --git a/src/test/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQueryElasticTest.java b/src/test/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQueryElasticTest.java index 720a4225a..bf95d0562 100644 --- a/src/test/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQueryElasticTest.java +++ b/src/test/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQueryElasticTest.java @@ -11,7 +11,6 @@ import com.fasterxml.jackson.core.JsonGenerator; import org.junit.Test; import java.io.IOException; -import java.io.StringWriter; import static org.assertj.core.api.Assertions.assertThat; @@ -28,8 +27,7 @@ public class DefaultOrmQueryElasticTest extends BaseElasticTest { SpiExpressionList whereExpressions = spiQuery.getWhereExpressions(); - StringWriter sb = new StringWriter(); - ElasticExpressionContext context = context(sb); + ElasticExpressionContext context = context(); JsonGenerator json = context.json(); json.writeStartObject(); json.writeFieldName("filter"); @@ -37,9 +35,8 @@ public class DefaultOrmQueryElasticTest extends BaseElasticTest { whereExpressions.writeElastic(context); json.writeEndObject(); - context.flush(); - assertThat(sb.toString()).isEqualTo("{\"filter\":{\"term\":{\"customer.name\":\"Rob\"}}}"); + assertThat(context.flush()).isEqualTo("{\"filter\":{\"term\":{\"customer.name\":\"Rob\"}}}"); } @Test @@ -52,13 +49,10 @@ public class DefaultOrmQueryElasticTest extends BaseElasticTest { SpiQuery spiQuery = (SpiQuery)query; - StringWriter sb = new StringWriter(); - ElasticExpressionContext context = context(sb); - + ElasticExpressionContext context = context(); spiQuery.writeElastic(context); - context.flush(); - assertThat(sb.toString()).isEqualTo("{\"fields\":[\"status\",\"customer.name\",\"details.product.id\"],\"query\":{\"filtered\":{\"filter\":{\"term\":{\"customer.name\":\"Rob\"}}}}}"); + assertThat(context.flush()).isEqualTo("{\"fields\":[\"status\",\"customer.name\",\"details.product.id\"],\"query\":{\"filtered\":{\"filter\":{\"term\":{\"customer.name\":\"Rob\"}}}}}"); } @Test