Skip to content

Additional test #836

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

Closed
wants to merge 3 commits into from
Closed

Additional test #836

wants to merge 3 commits into from

Conversation

bojanv55
Copy link

additional test for case when deserializing object reference. One of them works after applying my fix. other still doesn't work since there is additional nested Class.

@cowtowncoder
Copy link
Member

I can have a look at test failures first. I think you may have found a valid problem wrt access, but it needs to be solved bit differently (this is done when collecting properties, and only if allowed by configuration).

@cowtowncoder
Copy link
Member

With a quick look at failing test, I am not sure it should work: problem being that languageId reference is to Lang, but Object Id info is only enabled for LanguageId subtype. As such, deserializer has no expectation of Object Id being included. But serializer does see LanguageId type, and assumes Object Is required.

Inclusion of Object Id could in theory be prevented by using declaration like:

        @JsonSerialize(typing=JsonSerialize.Typing.STATIC)
        public Lang getLanguageId() {
            return languageId;
        }

which forces serializer to use static (declared) type for serialization; but unfortunately interface Lang does not contain enough information for serialization to succeed.

So I think the solution here would be to use JsonIdentityInfo in Lang. Object Id system is not designed to support subtypes that may or may not have Object Id.

@cowtowncoder
Copy link
Member

Part of suggested fixes make sense, and are included in #881 fix (cast problem; delegating of 'getCreatorIndex()'). But as I said earlier, I do not think the main part of the fix is correct for general case.

@cowtowncoder
Copy link
Member

As per my earlier comments, I think fixes that are merged (related to casts) are good; but that the test is unfortunately not valid as it assumes automatic casting from super-types, but without type information being available.

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

Successfully merging this pull request may close these issues.

2 participants