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

Use of @optional attribute while serializing #1541

Closed
tchaloupka opened this issue Aug 4, 2016 · 7 comments · Fixed by #2405
Closed

Use of @optional attribute while serializing #1541

tchaloupka opened this issue Aug 4, 2016 · 7 comments · Fixed by #2405

Comments

@tchaloupka
Copy link
Contributor

Lets have this code:

import std.stdio;
import std.typecons;
import vibe.data.json;

struct Bar { }

struct Foo
{
        long id;
        @optional Bar[] bars;
        @optional string name;
        @optional Nullable!long nullId;
}

void main()
{
        auto foo = Foo(1);
        writeln(foo);
        writeln(foo.serializeToJson());
        writeln(foo.serializeToJsonString());
}

It outputs:

Foo(1, [], "", Nullable.null)
{"nullId":null,"name":"","id":1,"bars":[]}
{"id":1,"bars":[],"name":"","nullId":null}

It would be nice if the serializer wont serialize fields that has initial value and are marked as @optional.

So we can have just this at the output:

{"id":1}

It could save a lot of data needed to be written to the output stream of REST API for example.

@deviator
Copy link
Contributor

deviator commented Aug 18, 2016

I think optional in and optional out must be separate. For example:

struct Foo
{
    @optional!In string bar;
    @optional!Out string baz;
}

{"baz":"ok"} valid json for this struct, but output json can be {"bar":"some"}, and not valid for input.
It can be useful in some cases.

@zunkree
Copy link

zunkree commented Aug 22, 2016

I believe non parameterized form will be little bit better:

struct Foo
{
    @optionalIn string bar;
    @optionalOut string baz;
}

because this @optional already parameterized with SerializationPolicy. Also, it is not so difficult to add such behavior I hope, but I need to figure out first how to skip fields during serialization.

@etcimon
Copy link
Contributor

etcimon commented Sep 13, 2016

Add here: https://github.com/rejectedsoftware/vibe.d/blob/71123ecb3d21a8eb2860fad364aedc11927f1483/source/vibe/data/serialization.d#L479

    static if (hasAttributeL!(OptionalAttribute, TA))
        if (vt == typeof(vt).init) continue;

@crimaniak
Copy link

crimaniak commented Nov 11, 2016

Instead of (or additional to) annotation it's possible to make special rendering case for Nullable because it's close related.

tchaloupka added a commit to tchaloupka/vibe.d that referenced this issue Dec 31, 2016
@tchaloupka
Copy link
Contributor Author

Made a PR for this. Not sure what to do with Nullable as mentioned by @crimaniak.
Handle it as a both way optional by default?

@zunkree
Copy link

zunkree commented Feb 20, 2017

Any news?

@s-ludwig
Copy link
Member

There is #1650, which has a working implementation, but the API should still be improved (see my last comments). I'm not sure about the best naming scheme, though, which is why I didn't offer a suggestion yet, so any ideas would be welcome.

tchaloupka added a commit to tchaloupka/vibe.d that referenced this issue Mar 14, 2017
tchaloupka added a commit to tchaloupka/vibe.d that referenced this issue Apr 8, 2017
tchaloupka added a commit to tchaloupka/vibe.d that referenced this issue May 14, 2017
tchaloupka added a commit to tchaloupka/vibe.d that referenced this issue Mar 2, 2019
WebFreak001 added a commit to WebFreak001/vibe.d that referenced this issue Jan 8, 2020
Fixes vibe-d#1541 and is an alternative, simpler implementation of vibe-d#1650
WebFreak001 added a commit to WebFreak001/vibe.d that referenced this issue Feb 19, 2020
Fixes vibe-d#1541 and is an alternative, simpler implementation of vibe-d#1650
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants