Hi,
I'm working on cloud-server#com.cloud.api.ApiDispatcher and related classes and trying to do some refactoring here. But I just had a couple of questions to make sure I'm not missing anything important. 1* There are several instance methods, but then there is this #processParameters that is static. It doesn't seem a method that I would make static. It seems more like someone wanted to call it from another class (perhaps AutoScaleManagerImpl) and instead of getting the ApiDispatcher object, they just made the method static, which I think would be wrong. Are there good reasons for this method to be static? Further, from this methods they needed to call #doAccessChecks but in this case, instead of making #doAccessChecks also static, they added a getInstance... which we can avoid if no method is static in the first place. The getInstance is also strange, but instead of fixing that it's better to just remove it since we will not need it: we should use Spring for injecting ApiDispatcher instead. Is there are reason not to do the change I propose? Am I missing something? 2* About the parameters received in the HTTP request, we have two methods for "fixing" them for our needs. First, in ApiServer#verifyRequest, we have a Map<String, String[]> but we never need to have an array of Strings, so the code assumes the array will always have 1 item, and always uses params[0] without even checking (the array could have 0 or 10 Strings). All of this also happens along ApiServlet. Later in ApiServer#handleRequest we do a first fix in the Map (which doesn't seem to fit well in this method). Actually we move the values to a Map<String, String> to stop having arrays. And from that point we use only the new Map. And finally, later in Apidispatcher we do a final fix in the Map: unpackedParams = cmd.unpackParams(params); So that params have values already lowercase and format validated. So unless someone knows reason to keep it this way, I want to fix the Map of params from the very beginning once and for all in a separate methods. >From that point all params will be checked, lower case and Map<String, String> without 1-item arrays... so future methods coming later don't need to worry about these details. Is there a reason doing this could be wrong? Is there any point in the middle where it will brake if my values are Strings instead of Arrays of Strings? Thanks. Cheers Antonio Fornie MCE at Schuberg Philis
