Remove use of java.util.Properties for URL query parsing utility about openrefine HOT 6 CLOSED

Hi, how do you recommend we change ImportingUtilities to accommodate the new changes?

If we are unable to modify the existing methods through refactoring, we could potentially face significant code duplication issues. For instance, in the case of the retrieveContentFromPostRequest method, it appears that only a single line of code needs to be modified within a method spanning 227 lines.

Any suggestions?

from openrefine.

from openrefine.

Okay, I think the problem is that the code that needs to be changed is nested very deep inside retrieveContentFromPostRequest, so I don't think that a refactoring of it would help that much to avoid code duplication.

Is it acceptable to modify the public method retrieveContentFromPostRequest as long as it doesn't change the signature and functionality of the method?

If so, my solution would be create a new method that takes Map<String,String> and from the original method simply convert Properties parameters to a Map<String, String> and call the new method. Like this:

// Original method
static public void retrieveContentFromPostRequest(
            HttpServletRequest request,
            Properties parameters,
            File rawDataDir,
            ObjectNode retrievalRecord,
            final Progress progress) throws IOException, FileUploadException {

        Map<String, String> parametersMap = new HashMap<>();
        for (String key : parameters.stringPropertyNames()) {
            String value = parameters.getProperty(key);
            parametersMap.put(key, value);
        }
        retrieveContentFromPostRequest(request, parametersMap, rawDataDir, retrievalRecord, progress);
}

// New method which takes map instead of properties
static public void retrieveContentFromPostRequest(
            HttpServletRequest request,
            Map<String, String> parameters,
            File rawDataDir,
            ObjectNode retrievalRecord,
            final Progress progress) throws IOException, FileUploadException {

            // Same code as before

}

Is that a viable solution?

from openrefine.

Yes, exactly. I suspect you'll need to do that for multiple methods, so you might want to extract your property remapping code into a separate utility method that you can reuse. Pleased also add an @Deprecated annotation to the original method so that people know not to use it for new code.

from openrefine.

p.s. you can use Java 8 Streams to make your remapping a little more concise:

   private static Map<String, String> mapProps(Properties properties) {
       return properties.entrySet().stream().collect(Collectors.toMap(e -> (String) e.getKey(), e -> (String) e.getValue()));
   }

from openrefine.

Great, thanks for your tips, will do that!

from openrefine.