[jira] [Commented] (AVRO-2775) JacksonUtils: exception when calling toJsonNode()

2021-01-05 Thread Andy Le (Jira)


[ 
https://issues.apache.org/jira/browse/AVRO-2775?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17259306#comment-17259306
 ] 

Andy Le commented on AVRO-2775:
---

[~rskraba] I think allowing the behaviour when generating defaults is enough. 
But would you elaborate more on

{quote}For my point #2 above (with Age going in, but Map coming out), maybe 
it's enough to just have this clearly documented on the JsonProperties 
interface.
{quote}


> JacksonUtils: exception when calling toJsonNode() 
> --
>
> Key: AVRO-2775
> URL: https://issues.apache.org/jira/browse/AVRO-2775
> Project: Apache Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.9.2
>Reporter: Andy Le
>Assignee: Andy Le
>Priority: Major
>
> I've got a simple test as followed
> {code:java}
> public class TestJacksonUtils {
>   public static class Age{
> public int value = 9;
>   }
>   @Test
>   public void testToJson(){
> Map kv = new HashMap<>();
> kv.put("age", 9);
> JsonNode node1 = JacksonUtils.toJsonNode(kv); // -> This is OK
> Object obj = new Age();
> JsonNode node2 = JacksonUtils.toJsonNode(obj); // -> This will trigger an 
> exception
>   }
> }
> {code}
> When I ran the test:
> {noformat}
> org.apache.avro.AvroRuntimeException: Unknown datum class: class 
> org.apache.avro.util.internal.TestJacksonUtils$Age
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJson(JacksonUtils.java:87)
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJsonNode(JacksonUtils.java:48)
>   at 
> org.apache.avro.util.internal.TestJacksonUtils.testToJson(TestJacksonUtils.java:20)
> {noformat}
> I've read the code & tests for JacksonUtils. Instead of raising exceptions at 
> [line 
> #87|https://github.com/apache/avro/blob/master/lang/java/avro/src/main/java/org/apache/avro/util/internal/JacksonUtils.java#L87],
>  I see we can auto convert objects into maps, every thing's gonna fine.
> My question is:
> - Is raising exception acceptable?
> - Any other way to have `toJsonNode` for general objects?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AVRO-2775) JacksonUtils: exception when calling toJsonNode()

2021-01-05 Thread Ryan Skraba (Jira)


[ 
https://issues.apache.org/jira/browse/AVRO-2775?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17259025#comment-17259025
 ] 

Ryan Skraba commented on AVRO-2775:
---

Hello! That *is* a more compelling use case for me, thanks for the clear 
explanation.

So to summarize: permitting an "arbitrary POJO" as a JSON metadata (by 
modifying {{JacksonUtils}}) will enable Avro reflection to better determine 
default values in the generated schema for POJO.

That sounds alright for me!

I don't see backwards compatibility being a huge issue in this case for 
developers relying on that exception being thrown for wrong data.

For my point #2 above (with {{Age}} going in, but {{Map}} coming out), maybe 
it's enough to just have this clearly documented on the JsonProperties 
interface.  Or alternatively, only permit this behaviour when generating 
defaults via ReflectData? 

> JacksonUtils: exception when calling toJsonNode() 
> --
>
> Key: AVRO-2775
> URL: https://issues.apache.org/jira/browse/AVRO-2775
> Project: Apache Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.9.2
>Reporter: Andy Le
>Priority: Major
>
> I've got a simple test as followed
> {code:java}
> public class TestJacksonUtils {
>   public static class Age{
> public int value = 9;
>   }
>   @Test
>   public void testToJson(){
> Map kv = new HashMap<>();
> kv.put("age", 9);
> JsonNode node1 = JacksonUtils.toJsonNode(kv); // -> This is OK
> Object obj = new Age();
> JsonNode node2 = JacksonUtils.toJsonNode(obj); // -> This will trigger an 
> exception
>   }
> }
> {code}
> When I ran the test:
> {noformat}
> org.apache.avro.AvroRuntimeException: Unknown datum class: class 
> org.apache.avro.util.internal.TestJacksonUtils$Age
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJson(JacksonUtils.java:87)
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJsonNode(JacksonUtils.java:48)
>   at 
> org.apache.avro.util.internal.TestJacksonUtils.testToJson(TestJacksonUtils.java:20)
> {noformat}
> I've read the code & tests for JacksonUtils. Instead of raising exceptions at 
> [line 
> #87|https://github.com/apache/avro/blob/master/lang/java/avro/src/main/java/org/apache/avro/util/internal/JacksonUtils.java#L87],
>  I see we can auto convert objects into maps, every thing's gonna fine.
> My question is:
> - Is raising exception acceptable?
> - Any other way to have `toJsonNode` for general objects?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AVRO-2775) JacksonUtils: exception when calling toJsonNode()

2021-01-05 Thread Andy Le (Jira)


[ 
https://issues.apache.org/jira/browse/AVRO-2775?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17258991#comment-17258991
 ] 

Andy Le commented on AVRO-2775:
---

[~rskraba] My use case is: I want to extract default values for fields. Please 
see my story here: [https://github.com/anhldbk/avro-fork]

 

For the time being, the code below throws exceptions:

 
{code:java}

import org.apache.avro.Schema;
import org.apache.avro.reflect.ReflectData;

public class AvroPlay {
  public static class Age{
public int value = 9;
  }

  public static class Person{
public String name = "Andy";
public Age age = new Age();
  }

  public static void main(String[] args) {
ReflectData reflector = new ReflectData().setDefaultsGenerated(true);
Schema schema = reflector.getSchema(Person.class);
System.out.println(schema.toString(true));
  }
}

{code}

When I run it, I've got an exception of:

{noformat}
Exception in thread "main" org.apache.avro.AvroRuntimeException: Unknown datum 
class: class org.example.AvroPlay$Age
at 
org.apache.avro.util.internal.JacksonUtils.toJson(JacksonUtils.java:96)
at 
org.apache.avro.util.internal.JacksonUtils.toJsonNode(JacksonUtils.java:53)
at org.apache.avro.Schema$Field.(Schema.java:581)
at 
org.apache.avro.reflect.ReflectData.createSchema(ReflectData.java:747)
at 
org.apache.avro.specific.SpecificData$3.computeValue(SpecificData.java:328)
at 
org.apache.avro.specific.SpecificData$3.computeValue(SpecificData.java:325)
at java.lang.ClassValue.getFromHashMap(ClassValue.java:227)
at java.lang.ClassValue.getFromBackup(ClassValue.java:209)
at java.lang.ClassValue.get(ClassValue.java:115)
at 
org.apache.avro.specific.SpecificData.getSchema(SpecificData.java:339)
at org.example.AvroPlay.main(AvroPlay.java:18)
{noformat}

So is it reasonable to modify the behavior of 
`JacksonUtils.toJson(JacksonUtils.java:96)` a little bit?
 

 

> JacksonUtils: exception when calling toJsonNode() 
> --
>
> Key: AVRO-2775
> URL: https://issues.apache.org/jira/browse/AVRO-2775
> Project: Apache Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.9.2
>Reporter: Andy Le
>Priority: Major
>
> I've got a simple test as followed
> {code:java}
> public class TestJacksonUtils {
>   public static class Age{
> public int value = 9;
>   }
>   @Test
>   public void testToJson(){
> Map kv = new HashMap<>();
> kv.put("age", 9);
> JsonNode node1 = JacksonUtils.toJsonNode(kv); // -> This is OK
> Object obj = new Age();
> JsonNode node2 = JacksonUtils.toJsonNode(obj); // -> This will trigger an 
> exception
>   }
> }
> {code}
> When I ran the test:
> {noformat}
> org.apache.avro.AvroRuntimeException: Unknown datum class: class 
> org.apache.avro.util.internal.TestJacksonUtils$Age
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJson(JacksonUtils.java:87)
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJsonNode(JacksonUtils.java:48)
>   at 
> org.apache.avro.util.internal.TestJacksonUtils.testToJson(TestJacksonUtils.java:20)
> {noformat}
> I've read the code & tests for JacksonUtils. Instead of raising exceptions at 
> [line 
> #87|https://github.com/apache/avro/blob/master/lang/java/avro/src/main/java/org/apache/avro/util/internal/JacksonUtils.java#L87],
>  I see we can auto convert objects into maps, every thing's gonna fine.
> My question is:
> - Is raising exception acceptable?
> - Any other way to have `toJsonNode` for general objects?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AVRO-2775) JacksonUtils: exception when calling toJsonNode()

2020-05-06 Thread Ryan Skraba (Jira)


[ 
https://issues.apache.org/jira/browse/AVRO-2775?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100865#comment-17100865
 ] 

Ryan Skraba commented on AVRO-2775:
---

Hello!  Just a quick point of view on this -- if I understand correctly, the 
goal is to be able to pass a POJO (that Jackson can handle) as a property 
(metadata) value in a schema?

The code that would be exposed to the user would be something like this:

{code}
  Schema s = ...
  s.addProp("age", new Age(9));
  ...
  // later
  Map m = (Map) s.getObjectProp("age")
{code}

I don't see this being a great idea, as opposed to the current behaviour, which 
is to only support a specified list of permitted java types, and throw an 
exception otherwise.  

My reasoning is: 

# Turning a  POJO into JSON is Jackson-specific behaviour and I'm not sure we 
want to tie Avro behaviour to it.  It might not be a big deal (if we're not 
planning on replacing Jackson any time soon).  
# Currently, what we put into a property value is also what we'll get out... 
and I think will always be the expected behaviour for the user!

If we maintain the current behaviour, the user is responsible for turning the 
POJO {{Age}} into the permitted java types, which seems reasonable to me.

> JacksonUtils: exception when calling toJsonNode() 
> --
>
> Key: AVRO-2775
> URL: https://issues.apache.org/jira/browse/AVRO-2775
> Project: Apache Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.9.2
>Reporter: Andy Le
>Priority: Major
>
> I've got a simple test as followed
> {code:java}
> public class TestJacksonUtils {
>   public static class Age{
> public int value = 9;
>   }
>   @Test
>   public void testToJson(){
> Map kv = new HashMap<>();
> kv.put("age", 9);
> JsonNode node1 = JacksonUtils.toJsonNode(kv); // -> This is OK
> Object obj = new Age();
> JsonNode node2 = JacksonUtils.toJsonNode(obj); // -> This will trigger an 
> exception
>   }
> }
> {code}
> When I ran the test:
> {noformat}
> org.apache.avro.AvroRuntimeException: Unknown datum class: class 
> org.apache.avro.util.internal.TestJacksonUtils$Age
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJson(JacksonUtils.java:87)
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJsonNode(JacksonUtils.java:48)
>   at 
> org.apache.avro.util.internal.TestJacksonUtils.testToJson(TestJacksonUtils.java:20)
> {noformat}
> I've read the code & tests for JacksonUtils. Instead of raising exceptions at 
> [line 
> #87|https://github.com/apache/avro/blob/master/lang/java/avro/src/main/java/org/apache/avro/util/internal/JacksonUtils.java#L87],
>  I see we can auto convert objects into maps, every thing's gonna fine.
> My question is:
> - Is raising exception acceptable?
> - Any other way to have `toJsonNode` for general objects?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AVRO-2775) JacksonUtils: exception when calling toJsonNode()

2020-04-19 Thread Andy Le (Jira)


[ 
https://issues.apache.org/jira/browse/AVRO-2775?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17086919#comment-17086919
 ] 

Andy Le commented on AVRO-2775:
---

[~cutting] any other feedback?

> JacksonUtils: exception when calling toJsonNode() 
> --
>
> Key: AVRO-2775
> URL: https://issues.apache.org/jira/browse/AVRO-2775
> Project: Apache Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.9.2
>Reporter: Andy Le
>Priority: Major
>
> I've got a simple test as followed
> {code:java}
> public class TestJacksonUtils {
>   public static class Age{
> public int value = 9;
>   }
>   @Test
>   public void testToJson(){
> Map kv = new HashMap<>();
> kv.put("age", 9);
> JsonNode node1 = JacksonUtils.toJsonNode(kv); // -> This is OK
> Object obj = new Age();
> JsonNode node2 = JacksonUtils.toJsonNode(obj); // -> This will trigger an 
> exception
>   }
> }
> {code}
> When I ran the test:
> {noformat}
> org.apache.avro.AvroRuntimeException: Unknown datum class: class 
> org.apache.avro.util.internal.TestJacksonUtils$Age
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJson(JacksonUtils.java:87)
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJsonNode(JacksonUtils.java:48)
>   at 
> org.apache.avro.util.internal.TestJacksonUtils.testToJson(TestJacksonUtils.java:20)
> {noformat}
> I've read the code & tests for JacksonUtils. Instead of raising exceptions at 
> [line 
> #87|https://github.com/apache/avro/blob/master/lang/java/avro/src/main/java/org/apache/avro/util/internal/JacksonUtils.java#L87],
>  I see we can auto convert objects into maps, every thing's gonna fine.
> My question is:
> - Is raising exception acceptable?
> - Any other way to have `toJsonNode` for general objects?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AVRO-2775) JacksonUtils: exception when calling toJsonNode()

2020-04-11 Thread Andy Le (Jira)


[ 
https://issues.apache.org/jira/browse/AVRO-2775?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17081209#comment-17081209
 ] 

Andy Le commented on AVRO-2775:
---

[~cutting] 

>  Is it incompatible enough to worry about?

Do you think our current tests are enough to validate compatibility? On our 
mailing lists, I see so many concerns about backward compatibility. Do you 
think we need to make breaking changes with Avro 2.x ?



> JacksonUtils: exception when calling toJsonNode() 
> --
>
> Key: AVRO-2775
> URL: https://issues.apache.org/jira/browse/AVRO-2775
> Project: Apache Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.9.2
>Reporter: Andy Le
>Priority: Major
>
> I've got a simple test as followed
> {code:java}
> public class TestJacksonUtils {
>   public static class Age{
> public int value = 9;
>   }
>   @Test
>   public void testToJson(){
> Map kv = new HashMap<>();
> kv.put("age", 9);
> JsonNode node1 = JacksonUtils.toJsonNode(kv); // -> This is OK
> Object obj = new Age();
> JsonNode node2 = JacksonUtils.toJsonNode(obj); // -> This will trigger an 
> exception
>   }
> }
> {code}
> When I ran the test:
> {noformat}
> org.apache.avro.AvroRuntimeException: Unknown datum class: class 
> org.apache.avro.util.internal.TestJacksonUtils$Age
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJson(JacksonUtils.java:87)
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJsonNode(JacksonUtils.java:48)
>   at 
> org.apache.avro.util.internal.TestJacksonUtils.testToJson(TestJacksonUtils.java:20)
> {noformat}
> I've read the code & tests for JacksonUtils. Instead of raising exceptions at 
> [line 
> #87|https://github.com/apache/avro/blob/master/lang/java/avro/src/main/java/org/apache/avro/util/internal/JacksonUtils.java#L87],
>  I see we can auto convert objects into maps, every thing's gonna fine.
> My question is:
> - Is raising exception acceptable?
> - Any other way to have `toJsonNode` for general objects?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AVRO-2775) JacksonUtils: exception when calling toJsonNode()

2020-04-10 Thread Doug Cutting (Jira)


[ 
https://issues.apache.org/jira/browse/AVRO-2775?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17080845#comment-17080845
 ] 

Doug Cutting commented on AVRO-2775:


Perhaps it's a non-issue.  Today attempts to pass an arbitrary Java class to 
this method would fail.  An application could depend on this.  So changing it 
is marginally incompatible.  Is it incompatible enough to worry about?

> JacksonUtils: exception when calling toJsonNode() 
> --
>
> Key: AVRO-2775
> URL: https://issues.apache.org/jira/browse/AVRO-2775
> Project: Apache Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.9.2
>Reporter: Andy Le
>Priority: Major
>
> I've got a simple test as followed
> {code:java}
> public class TestJacksonUtils {
>   public static class Age{
> public int value = 9;
>   }
>   @Test
>   public void testToJson(){
> Map kv = new HashMap<>();
> kv.put("age", 9);
> JsonNode node1 = JacksonUtils.toJsonNode(kv); // -> This is OK
> Object obj = new Age();
> JsonNode node2 = JacksonUtils.toJsonNode(obj); // -> This will trigger an 
> exception
>   }
> }
> {code}
> When I ran the test:
> {noformat}
> org.apache.avro.AvroRuntimeException: Unknown datum class: class 
> org.apache.avro.util.internal.TestJacksonUtils$Age
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJson(JacksonUtils.java:87)
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJsonNode(JacksonUtils.java:48)
>   at 
> org.apache.avro.util.internal.TestJacksonUtils.testToJson(TestJacksonUtils.java:20)
> {noformat}
> I've read the code & tests for JacksonUtils. Instead of raising exceptions at 
> [line 
> #87|https://github.com/apache/avro/blob/master/lang/java/avro/src/main/java/org/apache/avro/util/internal/JacksonUtils.java#L87],
>  I see we can auto convert objects into maps, every thing's gonna fine.
> My question is:
> - Is raising exception acceptable?
> - Any other way to have `toJsonNode` for general objects?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AVRO-2775) JacksonUtils: exception when calling toJsonNode()

2020-04-09 Thread Andy Le (Jira)


[ 
https://issues.apache.org/jira/browse/AVRO-2775?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17080173#comment-17080173
 ] 

Andy Le commented on AVRO-2775:
---

[~cutting]

> changing this would permit them to include arbitrary Java data structures

It's correct. Such structures may be represented as Maps

> which might obscure a bug if this was not intended

Would you elaborate more? 

> JacksonUtils: exception when calling toJsonNode() 
> --
>
> Key: AVRO-2775
> URL: https://issues.apache.org/jira/browse/AVRO-2775
> Project: Apache Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.9.2
>Reporter: Andy Le
>Priority: Major
>
> I've got a simple test as followed
> {code:java}
> public class TestJacksonUtils {
>   public static class Age{
> public int value = 9;
>   }
>   @Test
>   public void testToJson(){
> Map kv = new HashMap<>();
> kv.put("age", 9);
> JsonNode node1 = JacksonUtils.toJsonNode(kv); // -> This is OK
> Object obj = new Age();
> JsonNode node2 = JacksonUtils.toJsonNode(obj); // -> This will trigger an 
> exception
>   }
> }
> {code}
> When I ran the test:
> {noformat}
> org.apache.avro.AvroRuntimeException: Unknown datum class: class 
> org.apache.avro.util.internal.TestJacksonUtils$Age
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJson(JacksonUtils.java:87)
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJsonNode(JacksonUtils.java:48)
>   at 
> org.apache.avro.util.internal.TestJacksonUtils.testToJson(TestJacksonUtils.java:20)
> {noformat}
> I've read the code & tests for JacksonUtils. Instead of raising exceptions at 
> [line 
> #87|https://github.com/apache/avro/blob/master/lang/java/avro/src/main/java/org/apache/avro/util/internal/JacksonUtils.java#L87],
>  I see we can auto convert objects into maps, every thing's gonna fine.
> My question is:
> - Is raising exception acceptable?
> - Any other way to have `toJsonNode` for general objects?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AVRO-2775) JacksonUtils: exception when calling toJsonNode()

2020-04-09 Thread Doug Cutting (Jira)


[ 
https://issues.apache.org/jira/browse/AVRO-2775?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17079561#comment-17079561
 ] 

Doug Cutting commented on AVRO-2775:


The only downside I can see is that for applications that only intend to use 
the types listed in 
[JsonProperties|https://avro.apache.org/docs/current/api/java/org/apache/avro/JsonProperties.html],
 changing this would permit them to include arbitrary Java data structures, 
which might obscure a bug if this was not intended.  Are there other 
compatibility risks to adding this functionality?

> JacksonUtils: exception when calling toJsonNode() 
> --
>
> Key: AVRO-2775
> URL: https://issues.apache.org/jira/browse/AVRO-2775
> Project: Apache Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.9.2
>Reporter: Andy Le
>Priority: Major
>
> I've got a simple test as followed
> {code:java}
> public class TestJacksonUtils {
>   public static class Age{
> public int value = 9;
>   }
>   @Test
>   public void testToJson(){
> Map kv = new HashMap<>();
> kv.put("age", 9);
> JsonNode node1 = JacksonUtils.toJsonNode(kv); // -> This is OK
> Object obj = new Age();
> JsonNode node2 = JacksonUtils.toJsonNode(obj); // -> This will trigger an 
> exception
>   }
> }
> {code}
> When I ran the test:
> {noformat}
> org.apache.avro.AvroRuntimeException: Unknown datum class: class 
> org.apache.avro.util.internal.TestJacksonUtils$Age
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJson(JacksonUtils.java:87)
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJsonNode(JacksonUtils.java:48)
>   at 
> org.apache.avro.util.internal.TestJacksonUtils.testToJson(TestJacksonUtils.java:20)
> {noformat}
> I've read the code & tests for JacksonUtils. Instead of raising exceptions at 
> [line 
> #87|https://github.com/apache/avro/blob/master/lang/java/avro/src/main/java/org/apache/avro/util/internal/JacksonUtils.java#L87],
>  I see we can auto convert objects into maps, every thing's gonna fine.
> My question is:
> - Is raising exception acceptable?
> - Any other way to have `toJsonNode` for general objects?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AVRO-2775) JacksonUtils: exception when calling toJsonNode()

2020-03-17 Thread Andy Le (Jira)


[ 
https://issues.apache.org/jira/browse/AVRO-2775?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17061014#comment-17061014
 ] 

Andy Le commented on AVRO-2775:
---

I also think we should refactor JacksonUtils to clean it a little bit.

For example, currently we have:

 
{code:java}
public static Object toObject(JsonNode jsonNode, Schema schema) {
  if (schema != null && schema.getType().equals(Schema.Type.UNION)) {
return toObject(jsonNode, schema.getTypes().get(0));
  }
  if (jsonNode == null) {
return null;
  } else if (jsonNode.isNull()) {
return JsonProperties.NULL_VALUE;
  } else if (jsonNode.isBoolean()) {
return jsonNode.asBoolean();
  }
  //. ..
}
{code}
 
We can have a better code the above:

{code:java}
public static Object toObject(JsonNode jsonNode, Schema schema) {
  if (schema != null && schema.getType().equals(Schema.Type.UNION)) {
return toObject(jsonNode, schema.getTypes().get(0));
  }
  
  if (jsonNode == null) {
return null;
  } 
  
  if (jsonNode.isNull()) {
return JsonProperties.NULL_VALUE;
  } 
  
  if (jsonNode.isBoolean()) {
return jsonNode.asBoolean();
  }

  // ..
}
{code}

[~sekikn] do you think it's worth to refactor such code?

> JacksonUtils: exception when calling toJsonNode() 
> --
>
> Key: AVRO-2775
> URL: https://issues.apache.org/jira/browse/AVRO-2775
> Project: Apache Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.9.2
>Reporter: Andy Le
>Priority: Major
>
> I've got a simple test as followed
> {code:java}
> public class TestJacksonUtils {
>   public static class Age{
> public int value = 9;
>   }
>   @Test
>   public void testToJson(){
> Map kv = new HashMap<>();
> kv.put("age", 9);
> JsonNode node1 = JacksonUtils.toJsonNode(kv); // -> This is OK
> Object obj = new Age();
> JsonNode node2 = JacksonUtils.toJsonNode(obj); // -> This will trigger an 
> exception
>   }
> }
> {code}
> When I ran the test:
> {noformat}
> org.apache.avro.AvroRuntimeException: Unknown datum class: class 
> org.apache.avro.util.internal.TestJacksonUtils$Age
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJson(JacksonUtils.java:87)
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJsonNode(JacksonUtils.java:48)
>   at 
> org.apache.avro.util.internal.TestJacksonUtils.testToJson(TestJacksonUtils.java:20)
> {noformat}
> I've read the code & tests for JacksonUtils. Instead of raising exceptions at 
> [line 
> #87|https://github.com/apache/avro/blob/master/lang/java/avro/src/main/java/org/apache/avro/util/internal/JacksonUtils.java#L87],
>  I see we can auto convert objects into maps, every thing's gonna fine.
> My question is:
> - Is raising exception acceptable?
> - Any other way to have `toJsonNode` for general objects?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AVRO-2775) JacksonUtils: exception when calling toJsonNode()

2020-03-16 Thread Andy Le (Jira)


[ 
https://issues.apache.org/jira/browse/AVRO-2775?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17060589#comment-17060589
 ] 

Andy Le commented on AVRO-2775:
---

Another problem is: currently JacksonUtils does NOT handle Short and Byte (see 
[this 
code|https://github.com/apache/avro/blob/master/lang/java/avro/src/main/java/org/apache/avro/util/internal/JacksonUtils.java#L82-L84]).
 So I'm thinking of creating a PR to address them all.

 

[~fokko] [~sekikn] Please review my issue. Thank you!

> JacksonUtils: exception when calling toJsonNode() 
> --
>
> Key: AVRO-2775
> URL: https://issues.apache.org/jira/browse/AVRO-2775
> Project: Apache Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.9.2
>Reporter: Andy Le
>Priority: Major
>
> I've got a simple test as followed
> {code:java}
> public class TestJacksonUtils {
>   public static class Age{
> public int value = 9;
>   }
>   @Test
>   public void testToJson(){
> Map kv = new HashMap<>();
> kv.put("age", 9);
> JsonNode node1 = JacksonUtils.toJsonNode(kv); // -> This is OK
> Object obj = new Age();
> JsonNode node2 = JacksonUtils.toJsonNode(obj); // -> This will trigger an 
> exception
>   }
> }
> {code}
> When I ran the test:
> {noformat}
> org.apache.avro.AvroRuntimeException: Unknown datum class: class 
> org.apache.avro.util.internal.TestJacksonUtils$Age
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJson(JacksonUtils.java:87)
>   at 
> org.apache.avro.util.internal.JacksonUtils.toJsonNode(JacksonUtils.java:48)
>   at 
> org.apache.avro.util.internal.TestJacksonUtils.testToJson(TestJacksonUtils.java:20)
> {noformat}
> I've read the code & tests for JacksonUtils. Instead of raising exceptions at 
> [line 
> #87|https://github.com/apache/avro/blob/master/lang/java/avro/src/main/java/org/apache/avro/util/internal/JacksonUtils.java#L87],
>  I see we can auto convert objects into maps, every thing's gonna fine.
> My question is:
> - Is raising exception acceptable?
> - Any other way to have `toJsonNode` for general objects?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)