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

fromObject breaks oneof content #1596

Closed
alexander-fenster opened this issue Apr 27, 2021 · 2 comments · Fixed by #1597
Closed

fromObject breaks oneof content #1596

alexander-fenster opened this issue Apr 27, 2021 · 2 comments · Fixed by #1597
Assignees

Comments

@alexander-fenster
Copy link
Contributor

protobuf.js v6.10.2

In a kind of special case when oneof contains both a primitive field and a message, fromObject that is run on the instance of the static object would break the oneof by populating the primitive field with its default value.

Example:

test.proto

syntax = "proto3";

message TargetSpend {
  int32 micros = 1;
}

message OneofContainer {
  oneof campaign_bidding_strategy {
    string bidding_strategy = 1;
    TargetSpend target_spend = 2;
  }
}

repro.js

const protobuf = require('protobufjs');
const protos = require('./test.js');

const root = protobuf.Root.fromJSON(require('./test.json'));
const OneofContainer = root.lookup('OneofContainer');

const request = new protos.OneofContainer();
const targetSpend = new protos.TargetSpend();
targetSpend.micros = 1234;
request.targetSpend = targetSpend;

console.log(request);

const object = OneofContainer.fromObject(request);

console.log(object);

Commands to reproduce:

echo {} > package.json
npm install protobufjs
npx pbjs -t static-module -o test.js test.proto
npx pbjs -t json -o test.json test.proto
node repro.js

Result:

OneofContainer { targetSpend: TargetSpend { micros: 1234 } }    // before .fromObject
OneofContainer {
  biddingStrategy: '',
  targetSpend: TargetSpend { micros: 1234 }
}   // after .fromObject - the oneof is broken

Note that it work properly when fromObject is applied to a plain object:

const request1 = {
  targetSpend: {
    micros: 1234,
  }
};
const object1 = OneofContainer.fromObject(request1);
console.log(object1); // works

(I'm going to look into this, filing an issue just in case)

@alexander-fenster alexander-fenster self-assigned this Apr 27, 2021
@alexander-fenster
Copy link
Contributor Author

Upd. - after some thinking, it does not really matter if a oneof has a mix of primitive and non-primitive types; .fromObject will still do the wrong thing by assigning all oneof members a value.

@alexander-fenster
Copy link
Contributor Author

Upd. - this only happens when a .fromObject method of a dynamically generated class is applied to an instance of a statically generated class (produced by pbjs).

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 a pull request may close this issue.

1 participant