[GitHub] [hbase] virajjasani commented on a change in pull request #2745: [HBASE-25246] Backup/Restore hbase cell tags.

[GitBox <gi...@apache.org>]

virajjasani commented on a change in pull request #2745:
URL: https://github.com/apache/hbase/pull/2745#discussion_r542373728



##########
File path: hbase-client/src/test/java/org/apache/hadoop/hbase/shaded/protobuf/TestProtobufUtil.java
##########
@@ -479,4 +488,59 @@ public void testRegionLockInfo() {
         + "\"sharedLockCount\":0"
         + "}]", lockJson);
   }
+
+  /**
+   * Test {@link ProtobufUtil#toCell(Cell, boolean)} and
+   * {@link ProtobufUtil#toCell(ExtendedCellBuilder, CellProtos.Cell, boolean)} conversion
+   * methods when it contains tags and encode/decode tags is set to true.
+   */
+  @Test
+  public void testCellConversionWithTags() {
+
+    Cell cell = getCellWithTags();
+    CellProtos.Cell protoCell = ProtobufUtil.toCell(cell, true);
+    assertNotNull(protoCell);
+
+    Cell decodedCell = getCellFromProtoResult(protoCell, true);
+    List<Tag> decodedTags = PrivateCellUtil.getTags(decodedCell);
+    assertEquals(1,  decodedTags.size());
+    Tag decodedTag = decodedTags.get(0);
+    assertEquals(TAG_TYPE, decodedTag.getType());
+    assertEquals(TAG_STR, Tag.getValueAsString(decodedTag));
+  }
+
+  private Cell getCellWithTags() {
+    Tag tag = new ArrayBackedTag(TAG_TYPE, TAG_STR);
+    ExtendedCellBuilder cellBuilder = ExtendedCellBuilderFactory.create(CellBuilderType.DEEP_COPY);
+    cellBuilder.setRow(Bytes.toBytes("row1"));
+    cellBuilder.setFamily(Bytes.toBytes("f1"));
+    cellBuilder.setQualifier(Bytes.toBytes("q1"));
+    cellBuilder.setValue(Bytes.toBytes("value1"));
+    cellBuilder.setType(Cell.Type.Delete);
+    cellBuilder.setTags(Collections.singletonList(tag));
+    return cellBuilder.build();
+  }
+
+  private Cell getCellFromProtoResult(CellProtos.Cell protoCell, boolean decodeTags) {
+    ExtendedCellBuilder decodedBuilder =
+      ExtendedCellBuilderFactory.create(CellBuilderType.DEEP_COPY);
+    return ProtobufUtil.toCell(decodedBuilder, protoCell, decodeTags);
+  }
+
+  /**
+   * Test {@link ProtobufUtil#toCell(Cell, boolean)} and
+   * {@link ProtobufUtil#toCell(ExtendedCellBuilder, CellProtos.Cell, boolean)} conversion
+   * methods when it contains tags and encode/decode tags is set to false.
+   */
+  @Test
+  public void testCellConversionWithoutTags() {
+    Cell cell = getCellWithTags();
+    CellProtos.Cell protoCell =
+      ProtobufUtil.toCell(cell, false);
+    assertNotNull(protoCell);
+
+    Cell decodedCell = getCellFromProtoResult(protoCell, false);

Review comment:
       It seems we have good coverage, maybe one more possibility: encode with `true` and decode with `false` should also result in no tags available in decoded cell. Not blocker but good to have this test?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org