Response below... On Fri, May 26, 2017 at 11:49 AM, Jinmei Liao <jil...@pivotal.io> wrote:
> John, thank you for the detailed response. So the difference between these > two are the way request url is obtained, the RestHttpOperationInvoker > obtained the url from the returned LinkIndex map, while this > SimpleHttpOperationInvoker construct the url itself and eventually gets > something like "...//management/commands/cmd=xyz". > > Is that basically it? > > Essentially, yes. The SimpleHttpOperationInvoker only ever uses the single, fixed Controller endpoint (/management/commands/cmd=xyz) where as the RestHttpOperationInvoker attempts to the corresponding URL for the *Gfsh* command in the Index. The RestHttpOperationInvoker delegates to the SimpleHttpOperationInvoker when the *Gfsh* command is not represented in the Index, i.e. does not have a dedicated URL. > On Fri, May 26, 2017 at 11:33 AM, John Blum <jb...@pivotal.io> wrote: > > > Hi Jinmei- > > > > *> Do we know why in our admin rest api, we have these two kind > > of OpeationInvokers* > > > > Yes. > > > > The Javadoc > > <https://github.com/apache/geode/blob/rel/v1.1.1/geode- > > core/src/main/java/org/apache/geode/management/internal/web/shell/ > > SimpleHttpOperationInvoker.java#L33-L34> > > [1] > > somewhat explains the reason for this, but... > > > > In a nutshell, the Management (or "Admin") REST-like API has 2 type of > > REST service endpoints. > > > > First, are "Well-Known > > <https://github.com/apache/geode/blob/rel/v1.1.1/geode- > > core/src/main/java/org/apache/geode/management/internal/web/controllers/ > > ShellCommandsController.java#L143-L294>" > > [2] with actual *Spring Web MVC* Controller Endpoints corresponding to > each > > of the *Gfsh* commands (e.g. `create region > > <https://github.com/apache/geode/blob/rel/v1.1.1/geode- > > core/src/main/java/org/apache/geode/management/internal/web/controllers/ > > RegionCommandsController.java#L156-L228>` > > [3]). > > > > Then, there is a "Catch-all > > <https://github.com/apache/geode/blob/rel/v1.1.1/geode- > > core/src/main/java/org/apache/geode/management/internal/web/controllers/ > > ShellCommandsController.java#L68-L70>" > > endpoint [4], which is what the SimpleHttpOperationInvoker > > <https://github.com/apache/geode/blob/rel/v1.1.1/geode- > > core/src/main/java/org/apache/geode/management/internal/web/shell/ > > SimpleHttpOperationInvoker.java#L49> > > "invokes" [5]. The idea behind this endpoint was, often times, someone > > would add a new command but forget to update the Admin REST API to match, > > with "explicit" support (e.g. both this [2] and this [3]) for the new > > command. > > > > While this later approach works, it is not highly recommended and is > > certainly not very "REST-like", much less "REST-ful". > > > > In fact, the former, "Well-Known" endpoints I setup separately and > > individually in order to eventually introduce proper "Resource > > abstractions" for each of the REST API service endpoints for each of the > > *Gfsh* commands rather than the very use of HTTP Request Parameters to > > naively "reconstruct" the Gfsh command-line syntax that is largely still > in > > place today. However, I never got that far before the priorities > changed. > > > > So, *Gfsh* selects which HTTP-based OperationInvoker to use based on the > > availability of the command in the Admin REST API "Index" [2]. > > > > Anyway, there is much to know in terms of the reasons why this API was > > designed the way it was at the time. I'd say it still needs a lot of > work > > realize the full vision I had for it when it was created, something I > > documented quite well and gave to the leadership team at that time. > > > > If you have more questions, let's talk live. > > > > Cheers, > > John > > > > > > [1] > > https://github.com/apache/geode/blob/rel/v1.1.1/geode- > > core/src/main/java/org/apache/geode/management/internal/web/shell/ > > SimpleHttpOperationInvoker.java#L33-L34 > > [2] > > https://github.com/apache/geode/blob/rel/v1.1.1/geode- > > core/src/main/java/org/apache/geode/management/internal/web/controllers/ > > ShellCommandsController.java#L143-L294 > > [3] > > https://github.com/apache/geode/blob/rel/v1.1.1/geode- > > core/src/main/java/org/apache/geode/management/internal/web/controllers/ > > RegionCommandsController.java#L156-L228 > > [4] > > https://github.com/apache/geode/blob/rel/v1.1.1/geode- > > core/src/main/java/org/apache/geode/management/internal/web/controllers/ > > ShellCommandsController.java#L68-L70 > > [5] > > https://github.com/apache/geode/blob/rel/v1.1.1/geode- > > core/src/main/java/org/apache/geode/management/internal/web/shell/ > > SimpleHttpOperationInvoker.java#L49 > > > > > > On Fri, May 26, 2017 at 9:59 AM, Jinmei Liao <jil...@pivotal.io> wrote: > > > > > Hi, team, > > > > > > Do we know why in our admin rest api, we have these two kind of > > > OpeationInvokers, it looks like when RestHttpOperationInvoker failed to > > > find the resource, it will use the SimpleHttpOperationInvoker to try > > again, > > > but shouldn't that also fail as well? > > > > > > Here is some of the logic I found in the code. I am wondering why this > is > > > necessary. > > > > > > ..... > > > if( link is not available){ > > > throw new RestApiCallForCommandNotFoundException( > > > > > > String.format("No REST API call for command (%1$s) was found!", > > > command.getInput())); > > > } > > > > > > somewhere else in the process: > > > > > > catch (RestApiCallForCommandNotFoundException e) { > > > > > > SimpleHttpOperationInvoker.processCommand() > > > > > > } > > > > > > > > > -- > > > Cheers > > > > > > Jinmei > > > > > > > > > > > -- > > -John > > john.blum10101 (skype) > > > > > > -- > Cheers > > Jinmei > -- -John john.blum10101 (skype)
