[GitHub] [helix] jiajunwang commented on a change in pull request #1499: Trim strings in znrecord fields for REST json content

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

jiajunwang commented on a change in pull request #1499:
URL: https://github.com/apache/helix/pull/1499#discussion_r514893405



##########
File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/AbstractHelixResource.java
##########
@@ -83,7 +83,8 @@ public HelixDataAccessor getDataAccssor(String clusterName) {
 
   protected static ZNRecord toZNRecord(String data)
       throws IOException {
-    return OBJECT_MAPPER.reader(ZNRecord.class).readValue(data);
+    ZNRecord record = OBJECT_MAPPER.readValue(data, ZNRecord.class);
+    return ZNRecordUtil.trimFields(record);

Review comment:
       Here's an interesting problem.
   Since the rest is not covering all of our APIs, so assuming there is a user use java API sets " test" = 123. Then in the UI, user got "test" = 123 (due to this trim). Then user modifies it to be "test" = 456. After this change, in ZK, we now have both keys.
   
   " test" = 123
   "test" = 456
   
   To who reads the ZK directly, he or she does not know which one is the right value.
   To who reads it from UI, he or she will either get a duplicate key exception or see a random value depends on which value overwrites the other one.
   
   I think this problem is hard to address.
   That's why I think we still should try to fix the UI : )




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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org