gerrit/gerrit-gwtui/src
David Ostrovsky a4bccbfafe PluginAPI: Don't convert to String in RestApi.get() method
RestApi helper class makes use of ActionContext that makes use of
RestApi in Gerrit core to route Rest calls. Because ActionContext was
optimized for invocations from JavaScript plugin hooks, it's converting
NativeString to String.

The easiest way to see why this behaviour is needed is to investigate
the call graph:

  ActionButton => ActionContext.call() => ActionContext.get() =>
  RestApiCore.get() => return NativeString.asString() =>
  alert() called from JavaSrcipt that dump string content.

An example of this approach is implemented in cookbook-plugin:

1  function onSayHelloProject(c) {
2    var f = c.textfield();
3    var t = c.checkbox();
4    var b = c.button('Say hello', {onclick: function(){
5      c.call(
6        {message: f.value, french: t.checked},
7        function(r) {
8          c.hide();
9          window.alert(r);
10         c.refresh();
11       });
12   }});

Without this implicit conversion the output on the line 9 in the
example above would not be: "Hello dude..." but "[Object]".

While this implicit conversion is needed and correct in this use case,
when calling Rest API invocations from ActionButton, it is not wanted
and wrong when called from normal plugin screen through RestApiPlugin,
that makes itself use of the same implementation of ActionContext, to
route the RestAPI calls to RestApiCore implementation. The call graph:

  PluginScreen => RestApiPlugin.get() => ActionContext.get() =>
  RestApiCore.get() => return NativeString.asString() => type error

That's because in this use case, no implicit conversion should happen,
as the callback expects NativeString and not String, e.g.:

  private void retrieveGerritVersion(final Screen screen) {
    new RestApi("config").view("server").view("version")
    .get(new AsyncCallback<NativeString>() {
      @Override
      public void onSuccess(NativeString r) {
        screen.setPageTitle("cookbook index@" + r.asString());
      }
  [...]

Conclusion: while correct and expected in one use case, it's wrong and
leads to breakage in another use case. To rectify it, avoid conversion
when called from RestApiPlugin, as it's never wanted:

  PluginScreen => RestApiPlugin.get() => ActionContext.get2() =>
  RestApiCore.get() => return NativeString

The same problem is fixed for the other Rest methods
{put|post|delete}.

Change-Id: Id6c042478cab797838c1808602712a48039ec062
2015-06-29 15:08:17 +02:00
..
main/java PluginAPI: Don't convert to String in RestApi.get() method 2015-06-29 15:08:17 +02:00
test Simplify CodeMirror wrapper JSNI 2014-12-29 21:25:29 -05:00