rmdmattingly commented on code in PR #4937:
URL: https://github.com/apache/hbase/pull/4937#discussion_r1085636014


##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/OnlineLogRecord.java:
##########
@@ -36,10 +46,48 @@
 @InterfaceStability.Evolving
 final public class OnlineLogRecord extends LogEntry {
 
+  private static final Logger LOG = 
LoggerFactory.getLogger(OnlineLogRecord.class.getName());
+
+  private static Optional<JsonElement> serializeCatchAll(Operation operation) {
+    try {
+      return Optional.of(JsonParser.parseString(operation.toJSON()));
+    } catch (Exception e) {
+      LOG.warn("Suppressing exception during OnlineLogRecord serialization 
with operation {}",
+        operation, e);
+      return Optional.empty();
+    }
+  }
+
   // used to convert object to pretty printed format
   // used by toJsonPrettyPrint()
-  private static final Gson GSON =
-    
GsonUtil.createGson().setPrettyPrinting().registerTypeAdapter(OnlineLogRecord.class,
+  private static final Type OPERATION_LIST =
+    TypeToken.getParameterized(List.class, Operation.class).getType();
+  private static final Type OPERATION_MAYBE =
+    TypeToken.getParameterized(Optional.class, Operation.class).getType();
+  private static final Type OPERATION_LIST_MAYBE =
+    TypeToken.getParameterized(Optional.class, 
OPERATION_LIST.getClass()).getType();
+  private static final JsonElement EMPTY_NODE = 
JsonParser.parseString(HConstants.EMPTY_STRING);
+  private static final Gson GSON = GsonUtil.createGson().setPrettyPrinting()
+    .registerTypeAdapter(OPERATION_MAYBE.getClass(),
+      (JsonSerializer<
+        Optional<Operation>>) (operationMaybe, type, jsonSerializationContext) 
-> operationMaybe
+          .map(operation -> 
serializeCatchAll(operation).orElse(EMPTY_NODE)).orElse(EMPTY_NODE))
+    .registerTypeAdapter(OPERATION_LIST_MAYBE.getClass(), (JsonSerializer<
+      Optional<List<Operation>>>) (operationsMaybe, type, 
jsonSerializationContext) -> {
+        if (!operationsMaybe.isPresent()) {
+          return EMPTY_NODE;
+        }
+        JsonObject jsonObj = new JsonObject();
+        final AtomicInteger i = new AtomicInteger(0);
+        for (Operation operation : operationsMaybe.get()) {
+          serializeCatchAll(operation).ifPresent(json -> {
+            jsonObj.add(String.valueOf(i), json);
+            i.incrementAndGet();
+          });
+        }
+        return jsonObj;
+      })

Review Comment:
   Agreed — I've pushed some changes that make this much cleaner by defining a 
gson implementation to be used in the OnlineLogRecord serialization. I also 
added unit tests to validate the basic outputs.
   
   A benefit of this approach is that we can also stop the isEmpty based 
removal of operation nodes in the json output



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to