You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/07/30 07:25:24 UTC

[GitHub] [geode] jinmeiliao opened a new pull request #5411: GEODE-8372: Configure CMS to send UTF-8 regardless of JVM default

jinmeiliao opened a new pull request #5411:
URL: https://github.com/apache/geode/pull/5411


   


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



[GitHub] [geode] jinmeiliao merged pull request #5411: GEODE-8372: Configure CMS to send UTF-8 regardless of JVM default

Posted by GitBox <gi...@apache.org>.
jinmeiliao merged pull request #5411:
URL: https://github.com/apache/geode/pull/5411


   


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



[GitHub] [geode] demery-pivotal commented on a change in pull request #5411: GEODE-8372: Configure CMS to send UTF-8 regardless of JVM default

Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5411:
URL: https://github.com/apache/geode/pull/5411#discussion_r462585646



##########
File path: geode-management/src/main/java/org/apache/geode/management/api/RestTemplateClusterManagementServiceTransport.java
##########
@@ -73,6 +75,27 @@
   public RestTemplateClusterManagementServiceTransport(RestTemplate restTemplate) {
     this.restTemplate = restTemplate;
     this.restTemplate.setErrorHandler(DEFAULT_ERROR_HANDLER);
+
+    // configur the rest template to use a speciic jackson converter
+    List<HttpMessageConverter<?>> messageConverters = restTemplate.getMessageConverters();
+    MappingJackson2HttpMessageConverter jackson2HttpMessageConverter = messageConverters.stream()
+        .filter(MappingJackson2HttpMessageConverter.class::isInstance)
+        .map(MappingJackson2HttpMessageConverter.class::cast)
+        .findFirst().orElse(null);
+
+    if (jackson2HttpMessageConverter == null) {
+      jackson2HttpMessageConverter = new MappingJackson2HttpMessageConverter();
+      messageConverters.add(jackson2HttpMessageConverter);
+    }
+
+    jackson2HttpMessageConverter.setPrettyPrint(false);
+    // the client should use a mapper that would ignore unknown properties in case the server
+    // is a newer version than the client
+    jackson2HttpMessageConverter
+        .setObjectMapper(GeodeJsonMapper.getMapperIgnoringUnknownProperties());
+    // if we don't set the default charset here, the request will use ServletRequest's default
+    // charset which is ISO-8859

Review comment:
       I think this comment is overly specific. If we don't set the charset, the request will use some default, but it may not be ISO-8859. I've been able to change the default on my Mac and in docker images by:
   
   - setting the `LC_ALL` shell variable.
   - specifying the `file.encoding` JVM option (which I think overrides `LC_ALL`, but I'm not sure).
   
   Consider removing the comment. To me, `setDefaultCharset(UTF_8)` is clear enough without it.
   
   But if you want a comment, consider: "... which may not be UTF-8."




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