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

feat: accept unknown enum values in fromObject #1793

Merged
merged 2 commits into from
Aug 26, 2022

Conversation

alexander-fenster
Copy link
Contributor

In official protobuf implementations unknown numeric enum values are accepted and not discarded. Changing the behavior of fromObject to store unknown enum values. toObject will also dump them as is (without converting to a string, because it does not know the string representation for an unknown enum value).

I'm not changing verify which still won't accept unknown values.

Since the changes in the code generators are hard to read, here are some examples.

Imagine this proto:

syntax = "proto3";

enum E {
  DEFAULT = 0;
  KNOWN = 1;
}

message M {
  E a = 1;
  repeated E b = 2;
}

This is the diff in the generated code for fromObject and toObject, explanations are below.

--- original.js	2022-08-26 16:51:09.932392333 +0000
+++ changed.js	2022-08-26 17:23:27.797270346 +0000
@@ -3,6 +3,9 @@
   return d
   var m=new this.ctor
   switch(d.a){
+  default:
+  if(typeof(d.a)==="number"){m.a=d.a;break}
+  break
   case"DEFAULT":
   case 0:
   m.a=0
@@ -19,6 +22,7 @@
   for(var i=0;i<d.b.length;++i){
   switch(d.b[i]){
   default:
+  if(typeof(d.b[i])==="number"){m.b[i]=d.b[i];break}
   case"DEFAULT":
   case 0:
   m.b[i]=0
@@ -43,12 +47,12 @@
   d.a=o.enums===String?"DEFAULT":0
   }
   if(m.a!=null&&m.hasOwnProperty("a")){
-  d.a=o.enums===String?types[0].values[m.a]:m.a
+  d.a=o.enums===String?(types[0].values[m.a]===undefined?m.a:types[0].values[m.a]):m.a
   }
   if(m.b&&m.b.length){
   d.b=[]
   for(var j=0;j<m.b.length;++j){
-  d.b[j]=o.enums===String?types[1].values[m.b[j]]:m.b[j]
+  d.b[j]=o.enums===String?(types[1].values[m.b[j]]===undefined?m.b[j]:types[1].values[m.b[j]]):m.b[j]
   }
   }
   return d

Previously, this was the code generated for fromObject and toObject (comments added):

function M$fromObject(d){
  if(d instanceof this.ctor)
  return d
  var m=new this.ctor
  switch(d.a){      // for single enum values, there's no "default:" clause, it ignores unknown values
  case"DEFAULT":
  case 0:
  m.a=0
  break
  case"KNOWN":
  case 1:
  m.a=1
  break
  }
  if(d.b){
  if(!Array.isArray(d.b))
  throw TypeError(".M.b: array expected")
  m.b=[]
  for(var i=0;i<d.b.length;++i){
  switch(d.b[i]){
  default:
  case"DEFAULT":    // for repeated enum values, it replaces unknown values with default values
                    // to avoid having holes in array
  case 0:
  m.b[i]=0
  break
  case"KNOWN":
  case 1:
  m.b[i]=1
  break
  }
  }
  }
  return m
}
function M$toObject(m,o){
  if(!o)
  o={}
  var d={}
  if(o.arrays||o.defaults){
  d.b=[]
  }
  if(o.defaults){
  d.a=o.enums===String?"DEFAULT":0
  }
  if(m.a!=null&&m.hasOwnProperty("a")){
  d.a=o.enums===String?types[0].values[m.a]:m.a // will put undefined for unknown value
  }
  if(m.b&&m.b.length){
  d.b=[]
  for(var j=0;j<m.b.length;++j){
  d.b[j]=o.enums===String?types[1].values[m.b[j]]:m.b[j] // same here
  }
  }
  return d
}

With the changes, the following code will be produced:

function M$fromObject(d){
  if(d instanceof this.ctor)
  return d
  var m=new this.ctor
  switch(d.a){
  default:
  if(typeof(d.a)==="number"){m.a=d.a;break} // if an unknown value is a number, use it as is,
  break                                     // otherwise, ignore it since it's not an array
  case"DEFAULT":
  case 0:
  m.a=0
  break
  case"KNOWN":
  case 1:
  m.a=1
  break
  }
  if(d.b){
  if(!Array.isArray(d.b))
  throw TypeError(".M.b: array expected")
  m.b=[]
  for(var i=0;i<d.b.length;++i){
  switch(d.b[i]){
  default:
  if(typeof(d.b[i])==="number"){m.b[i]=d.b[i];break} // for arrays, use unknown numbers as is, but
  case"DEFAULT":                                     // don't leave holes
  case 0:
  m.b[i]=0
  break
  case"KNOWN":
  case 1:
  m.b[i]=1
  break
  }
  }
  }
  return m
}
function M$toObject(m,o){
  if(!o)
  o={}
  var d={}
  if(o.arrays||o.defaults){
  d.b=[]
  }
  if(o.defaults){
  d.a=o.enums===String?"DEFAULT":0
  }
  if(m.a!=null&&m.hasOwnProperty("a")){
  d.a=o.enums===String?(types[0].values[m.a]===undefined?m.a:types[0].values[m.a]):m.a
  // will put a number for unknown value
  }
  if(m.b&&m.b.length){
  d.b=[]
  for(var j=0;j<m.b.length;++j){
  d.b[j]=o.enums===String?(types[1].values[m.b[j]]===undefined?m.b[j]:types[1].values[m.b[j]]):m.b[j]
  // same here
  }
  }
  return d
}

@alexander-fenster alexander-fenster requested a review from bcoe August 26, 2022 17:26
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