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.

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

Decode avro message to standard JSON #247

Closed
d-rk opened this issue May 2, 2022 · 6 comments
Closed

Decode avro message to standard JSON #247

d-rk opened this issue May 2, 2022 · 6 comments

Comments

@d-rk
Copy link

d-rk commented May 2, 2022

Hello,

I recently found out, that it is now possible to use standard JSON as input to goavro 👍

As far as I can tell, that currently does not work the other way around, meaning when I decode an avro message with the same codec it does not generate standard JSON:

	codec, err := goavro.NewCodecForStandardJSON(`
        {
          "type": "record",
          "name": "myUnionType",
          "fields" : [
            { "name": "myField", "type": ["null", "string"]}
          ]
        }`)
	if err != nil {
		fmt.Println(err)
	}

	textual := []byte(`{"myField":"testValue"}`)
	native, _, err := codec.NativeFromTextual(textual)
	if err != nil {
		fmt.Println(err)
	}
	binary, err := codec.BinaryFromNative(nil, native)
	if err != nil {
		fmt.Println(err)
	}
	native, _, err = codec.NativeFromBinary(binary)
	if err != nil {
		fmt.Println(err)
	}
	textual, err = codec.TextualFromNative(nil, native)
	if err != nil {
		fmt.Println(err)
	}
	fmt.Println(string(textual))

This currently prints {"myField":{"string":"testValue"}}, but I expected it to be {"myField":"testValue"}.
Am I missing something?
Is the decoding not implemented, yet?
Are there plans to do so?

Thanks

@xmcqueen
Copy link
Contributor

Hi @d-rk, I had thought there was a good reason not to implement the decoding, but now that you have asked, none of the reason's seem very good any longer. I'm going to wait a bit longer and consider this. Also @karrick might have an opinion on this.

@xmcqueen
Copy link
Contributor

I've got some code for this in testing. I'll still wait a bit to see if @karrick or anybody can think of any hazards. I've done it in the safest way so it will probably be good. I made a new New* function for two-way json usage. The old one will still do exactly what it did before. Folks who want to go two-ways will be able to use the new New* function. I think this approach should be perfectly safe aside from bugs. I've got a full suite of tests on it already.

@xmcqueen
Copy link
Contributor

Can you go have a look at pr #249. It needs some reviewers.

@xmcqueen
Copy link
Contributor

I think we close this one now. I just merged the two-way json PR #249, and it is expected to provide the functionality you are describing.

@d-rk
Copy link
Author

d-rk commented Sep 7, 2022

Works as expected! Thank you @xmcqueen

@karrick
Copy link
Contributor

karrick commented Oct 27, 2022

This apparent double encoding encapsulation is required for union type support, and it was done in accordance with the Avro specification. The Avro specification requires values of union types to be represented as a JSON object, which itself declares which of the various union types this particular datum is. Removing this extra annoying element will break ability of clients to encode or decode schemas with union types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants