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

Protobuf serialization for Output, OutputFactory and OutputInfo #255

Merged
merged 12 commits into from
Aug 20, 2022

Conversation

Craigacp
Copy link
Member

@Craigacp Craigacp commented Aug 8, 2022

Description

This is the next chunk of protobuf serialization support, adding support for Output, OutputInfo, OutputFactory and their subclasses. Most of this PR is the 15k+ lines of generated protos, the handwritten code & proto schemas is a lot smaller.

Motivation

We need to move off Java serialization. See #226.

@Craigacp Craigacp added the squash-commits Squash the commits when merging this PR label Aug 8, 2022
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Aug 8, 2022
}

@Override
public OutputFactoryProto serialize() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you annotated this class with:

@ProtoSerializableClass(version = 0)

Then you could use the default serialization:

    @Override
    public OutputFactoryProto serialize() {
        return ProtoUtil.serialize(this);
    }

Copy link
Member

@pogren pogren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Output, OutputFactory, OutputInfo now extend ProtoSerializable
Major changes include:

  • everything has serialization and deserialization now - either using default/built-in mechanism when possible, or custom serialization when not - nothing tricksy in here that I could see.
  • A number of constructors were added to support deserialization - some of them with improved constraint checking
  • various equals and hashCode methods added presumably to support simplified unit testing of equals with original objects with their serialized then deserialized twins

My only quibble is that LabelFactory, AnomalyFactory, ClusteringFactory, MultiLabelFactory could use the default serialization with an annotation like this: @ProtoSerializableClass(version = 0). I can submit a PR if you want my local code for this.

@Craigacp
Copy link
Member Author

Sure. This PR isn't consistent with the others wrt the use of a static final CURRENT_VERSION field either but we can catch that up later.

@Craigacp Craigacp merged commit ceec5ee into oracle:main Aug 20, 2022
@Craigacp Craigacp deleted the output-serialization branch August 20, 2022 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement. squash-commits Squash the commits when merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants