From 6cb3dc839cf2c845a8d2f182c20fa68dcc1a66b6 Mon Sep 17 00:00:00 2001 From: juhyun seo Date: Fri, 19 Jan 2024 06:45:00 +0900 Subject: [PATCH] fix(protobuf): fix reseved field error in fields in nested messages (#9318) --- .../datahub/protobuf/model/ProtobufField.java | 17 ++-- .../protobuf/model/ProtobufFieldTest.java | 32 +++++++ .../extended_protobuf/messageD.proto | 89 ++++++++++++++++++ .../extended_protobuf/messageD.protoc | Bin 0 -> 8749 bytes 4 files changed, 132 insertions(+), 6 deletions(-) create mode 100644 metadata-integration/java/datahub-protobuf/src/test/resources/extended_protobuf/messageD.proto create mode 100644 metadata-integration/java/datahub-protobuf/src/test/resources/extended_protobuf/messageD.protoc diff --git a/metadata-integration/java/datahub-protobuf/src/main/java/datahub/protobuf/model/ProtobufField.java b/metadata-integration/java/datahub-protobuf/src/main/java/datahub/protobuf/model/ProtobufField.java index 5bb4101748..c3ede2e62c 100644 --- a/metadata-integration/java/datahub-protobuf/src/main/java/datahub/protobuf/model/ProtobufField.java +++ b/metadata-integration/java/datahub-protobuf/src/main/java/datahub/protobuf/model/ProtobufField.java @@ -277,13 +277,18 @@ public class ProtobufField implements ProtobufElement { messageType = messageType.getNestedType(value); } - if (pathList.get(pathSize - 2) == DescriptorProto.FIELD_FIELD_NUMBER - && pathList.get(pathSize - 1) != DescriptorProto.RESERVED_RANGE_FIELD_NUMBER - && pathList.get(pathSize - 1) != DescriptorProto.RESERVED_NAME_FIELD_NUMBER) { - return messageType.getField(pathList.get(pathSize - 1)); - } else { - return null; + int fieldIndex = pathList.get(pathList.size() - 1); + if (isFieldPath(pathList) + && pathSize % 2 == 0 + && fieldIndex < messageType.getFieldList().size()) { + return messageType.getField(fieldIndex); } + + return null; + } + + private boolean isFieldPath(List pathList) { + return pathList.get(pathList.size() - 2) == DescriptorProto.FIELD_FIELD_NUMBER; } private boolean isEnumType(List pathList) { diff --git a/metadata-integration/java/datahub-protobuf/src/test/java/datahub/protobuf/model/ProtobufFieldTest.java b/metadata-integration/java/datahub-protobuf/src/test/java/datahub/protobuf/model/ProtobufFieldTest.java index 9508f4778e..40d54a8651 100644 --- a/metadata-integration/java/datahub-protobuf/src/test/java/datahub/protobuf/model/ProtobufFieldTest.java +++ b/metadata-integration/java/datahub-protobuf/src/test/java/datahub/protobuf/model/ProtobufFieldTest.java @@ -323,4 +323,36 @@ public class ProtobufFieldTest { assertEquals("Zip code, alphanumeric", addressField.getDescription()); } + + @Test + public void nestedTypeReservedFieldsTest() throws IOException { + ProtobufDataset test = getTestProtobufDataset("extended_protobuf", "messageD"); + SchemaMetadata testMetadata = test.getSchemaMetadata(); + + SchemaField msg3Field13 = + testMetadata.getFields().stream() + .filter( + v -> + v.getFieldPath() + .equals( + "[version=2.0].[type=extended_protobuf_MyMsg]." + + "[type=extended_protobuf_MyMsg_Msg3].field3.[type=google_protobuf_StringValue].msg3_13")) + .findFirst() + .orElseThrow(); + + assertEquals("test comment 13", msg3Field13.getDescription()); + + SchemaField msg3Field14 = + testMetadata.getFields().stream() + .filter( + v -> + v.getFieldPath() + .equals( + "[version=2.0].[type=extended_protobuf_MyMsg]." + + "[type=extended_protobuf_MyMsg_Msg3].field3.[type=google_protobuf_StringValue].msg3_14")) + .findFirst() + .orElseThrow(); + + assertEquals("test comment 14", msg3Field14.getDescription()); + } } diff --git a/metadata-integration/java/datahub-protobuf/src/test/resources/extended_protobuf/messageD.proto b/metadata-integration/java/datahub-protobuf/src/test/resources/extended_protobuf/messageD.proto new file mode 100644 index 0000000000..4aaf80cf78 --- /dev/null +++ b/metadata-integration/java/datahub-protobuf/src/test/resources/extended_protobuf/messageD.proto @@ -0,0 +1,89 @@ +syntax = "proto3"; +package extended_protobuf; + +import "google/protobuf/wrappers.proto"; + +/* + MyMsg Message + */ +message MyMsg { + /* + Message 1 + */ + message Msg1 { + int32 msg1_id = 1; + } + Msg1 msg1_field = 1; + + /* + Message 2 + */ + message Msg2 { + int32 msg2_id = 1; + } + Msg2 msg2_field = 2; + + /* + Message 3 + */ + message Msg3 { + // test comment 1 + google.protobuf.Int64Value msg3_1 = 1; + // test comment 2 + google.protobuf.Int64Value msg3_2 = 2; + // test comment 3 + google.protobuf.Int64Value msg3_3 = 3; + // test comment 4 + google.protobuf.StringValue msg3_4 = 4; + // test comment 5 + reserved 5; + // test comment 6 + reserved 6; + + message Msg4 { + // msg4_1 comment + google.protobuf.Int32Value msg4_1 = 1; + // msg4_2 reserved + reserved 2; + // msg4_3 comment + google.protobuf.Int32Value msg4_3 = 3; + + message Msg5 { + // msg5_1 comment + google.protobuf.Int32Value msg5_1 = 1; + // msg5_2 comment + google.protobuf.Int32Value msg5_2 = 2; + // msg5_3 comment + google.protobuf.Int32Value msg5_3 = 3; + // msg5_4 comment + google.protobuf.Int32Value msg5_4 = 4; + // reserved comment + reserved 5; + // msg5_6 comment + google.protobuf.Int32Value msg5_6 = 6; + } + // msg5 comment + Msg5 msg5 = 4; + } + // test comment 7 + Msg4 msg4 = 7; + // test comment 8 + google.protobuf.StringValue msg3_8 = 8; + // test comment 9 + google.protobuf.StringValue msg3_9 = 9; + // test comment 10 + google.protobuf.StringValue msg3_10 = 10; + // test comment 11 + reserved 11; + // test comment 12 + google.protobuf.StringValue msg3_12 = 12; + // test comment 13 + google.protobuf.StringValue msg3_13 = 13; + // test comment 14 + google.protobuf.StringValue msg3_14 = 14; + // test comment 15 + google.protobuf.StringValue msg3_15 = 15; + } + // field 3 + Msg3 field3 = 3; +} \ No newline at end of file diff --git a/metadata-integration/java/datahub-protobuf/src/test/resources/extended_protobuf/messageD.protoc b/metadata-integration/java/datahub-protobuf/src/test/resources/extended_protobuf/messageD.protoc new file mode 100644 index 0000000000000000000000000000000000000000..03cb56b35314a849cb594321a119f3e05965ed3c GIT binary patch literal 8749 zcmbtZ%TgoB71g5xI9;-uW*a;qca5M|f-hgO@%x>nR}`BFq9Rv+h0j+(+KbEZ9Guv$-?NI%}Cr zmmSNtPI{+HR~_T>((H6IVn&{H-a8|_pMTEAidOHWWxh9BJyU+fMn2NROq53#qPjDf zpJ!~dWf}GmnOI0hYul^&Au@4K2FCVp6Oue37=uhMI|OLNlW~ED3Qc%IU$E4=WwnOj zBs~$}RP9c)eKtfWYN3+vac`A?$Xa~>#FVN?egTm(deII@vX}GUt)7xBD)y@hU{ey%vS4HBwr)d zoxeY030Yc*{4UD=HN~Ria(p2oXa73Rg+kqG@%7&6DXM|L<_^@$E^ipN!JBQ{?9|Ue z$>gV2=fbcV->@#PJI%9mn`f7o*IWgzwd)zql~#+3cU|5wyJqL3*~l<{ZrhjLw@XV6 z^P}0aMA^CSA~uenAn_W^iNq3PTsIrdE;4Y^vzu0%8|?<~bxqLWU8~orn_}jq*)}@Y zf>$@qubTEbw>sjl)dSB9tI<4d)(t{PbE9K&SiWf5w%OpQ`j5>99_(|&#xL@&)v~UT za$dLEjV38|NeMWn{T7tyXK{d7ms_VEqq@~F`9-g5!*kmJQ$jUPtdAxT-H0*Xw(Mrz zOygy<%Uke~5W*JWX+IqVqTVu^7iI^M98LnpLb32jz{f@pq4IgKfXM&FgHav`wPDqJ z7iQZQA*PfpA^sNLbT}%->@Lviv`ykNjZHoDfS zeMMzLL{UM!dui6Gf}qu;!s<}%w4EyIc2OW1ltpb@tMY1jvv#29D#vq0FTdA{YLTxW z;-$(r%9TT1+uE-2?ecC>)vH`76){_?>Du~!t*loWpI53-ofmJE(jixWtmtaB%F8;} z_A0v?&;YM1rJANz(_AZU?Cuw}(pH)SftSiP#&@+ntp>STIW5fI)Z^t%zNhLN+kjEl zwOy@tC=70DwGtWIEbELbyrSqeZDW5|(RpQGuav7QC(nvnbz@i2_S9kq_FZ-jT{P|HV`}z+)XoHt*9Fso)8Ze zVNmqLv?Qgrd5?Uae`h{hG?R%H^U6c2(8i zYa43yTfSSS@Ne?{stO`SrKSkez(Bb09uBPUS2YnhtyEKWeZNxE%B5Gx@&O_P*A?g% zMTE;G@|J2vE$fE_OK}hxNb`ej74t}R$%&Ri0jZ)mHfmup7(}fg(Se`5q;BnMTWV=T zB|c?>Inb)=D5sz~rcis(61x(G7z$+bcDD;GUvYfNDDCTGfg?5oVYy}0O*Din^bjmz^bphO0@Q4P z+G}>t(H!h66HA!-h;h<2+h~tyGe%$%y#QM#`a;ueH3T@?47$as(QDcKBb|Z7suzxU zhbVq-z~i=st-@%-7JL%h6*UsH%8q$yq8+-R*j2hj;F^Q+&(wQ}OS9Fquc>J@Oxvv6 zAcRN>dTj)u(Zp`mGg{ninbdH*=$cL}?@7TxZ@i>NXmG1-(x%k}I5qMBni{DoI;$@& zqTo(BdUXT4ajRn*jq5?Gh)a@1V{&{kGDA;EIi8Mu%Tg?s{L*=JCSOOMiOFYjoF1LY zOk`V3j>&O)bS9S~X)zg<1>F5F#?YVA05G(P6oI)q@mhYH3f zji0y_0q1X$4;g2Y6p2OT_?y_n*@u)g?3#40_G@XJjgZ6$i6ogjg7%x#18)?^Xj*0_ z9Ei%&50UlgEEctMg6wzTl#b(f^AAq#x6qG*zz<28Jp=tH(N|*ftSsmY6MgI<0QO33 zRPe6KL!F@F8dg<*L?QE zL?3$yfW79j#|X-r%N`@J*T9~NCoW6nh<|I&pZ~J)$u8g+$Nz= z6H0)}S#p}qVHGFHk!Eaqc3P}rGS-&|yoVdjSVAmYjG)^b9rs3Xqd7SxmcghjS&`1I zd2m+Z-HwDV*num3%XkzNEH@9MME`@I2S={Y0Z71s4{jbXg7ybD4;aCL5AHHZ%F-3? z!ngI%6S~yj%|E#N-$FkL0$1)bND}>Pe;Ei9ee59s_Sfz*zzE9M?lQm#?60S$PZHt) zlw<$o?18&+72Pj5)8=n+Il-0o$U7v>-tp0A{@;v^>|O75&*bkIyXc;+9Ets2eB&9* z{xWj|F(afi&{)w^q_9b+Bv(7@YRB+}=nn0y(9SyA1(pzIb2IU~HFLTvJd2%)c*sxL zsG#0#2;L)lf`(ed709{**`q#zECl|O6c9N%%Sb18wBmaH+2H4zOt9nVBP6=w>ttsf zfA7}G`Z}JcckATjoXaggBZ=sIeuG`SJ|AhnC3C%5tEVof*HkS~gJ2%}m@|Ef=WeXCB;JEgz^A zW+v~gR+#-;T$E$pEtCAG%hBDzM8@-8Ip&W(Sz3KMIr&>rB?Y&zUfx|LR4N5gDuvlU z#su4f%eHWTwgr!E;pmf#C(O3sEuO;tJuG+&yKv+!o}mk_&yE-U!qMtXLL~2Qoz)#F zIWm>N@7Pq_e@;w|;(2UpeCN-P>FaAG@?(_lCHVLniOBSXn^H6~OE;7_Hivj*fqmxO zP?8Xxon)ff@;&E%;%w-&DbTGz#Gx6v#8}n{2ld&fbax9XNt!7>1927biH`w(-Rkr$38vhI`P=2=UgZr`}8n|oyR^sU_JHeMM3YWPmk<8 z_31^S_SC0`yU6FU{V=_pN006gpMxGgn1=y1YmVe znhVANn0xkRFoxy1g;zcxUh|uP2#*IyZtxHwIl@DLU8=Fb2T+5FoiY z9|DX4a6SZx;S1^OL6`FZU>txiq-VYXy0g6CUj<`Ac(sVLC$6{gSW1q6B~1?E6_{J0 zu@oxwtBD7mdWs6GZwwD<9KbEygj)^ZpuP}-qxkiW0q2ZGhG{7+O6wz@d9i*L$EVmA zEJ^I06_ie-;Q;NdM=M?xFgC?fo(MvcPVn#y%%;D18J;Kz?u}RN1#U_Kpw{J;AXJ7^n(M&)Xrk4Z4xnYE<%9i^GQ=FFj2vemkgYcBchGLaX z{4g5O5z)#%klAy_9H*Rk<&zmFUf3N1^D-L?gg_vloM(M=ppbw5s(%h7^6A%o1|YT? z7|0GcKpw6R7$6^42Mmyxs{;lQ(Y3%pD1c$Mw64|$4A9zI8!$lYYi+;)rrrhy^20r# zb@q0^0IjvR0|w%R1_Lj=aulWYUGgD5+j#$q3oJ|rnWF0Y4mYEaSE9ad%po2$7f!My z$G;CeSPk$)V07<=RDY7`6T$Zb9^m^*{CjwSPlE1(f<-0qX!ZfEjY0tKuY(vxUrB`5 zAjMu8n+%FGMv+(Khkow`=b|JJcA{^B&PIHTQqt=M^vx|^Oicu102c90!rL9Nmu}u{ zm1_au2mm)jt&+YkY=&Cp2mm+HDm_4mZ{Gxb696WJM9_Qbp1&OeOaO2jfZW}`MrE8! zUZcnK?_bLS;wTVzLKNxhy~AJnK92%%XE6LEl1L_LIvkKMH)h}adqZF>xewGiSM5NjdCF(B4}_(UAk k5Yz*FV@SFGz3|R6VwoKHha$?w!Km|jHX@eCLGWqee-Q?U)Bpeg literal 0 HcmV?d00001