[forge-issues] [JBoss JIRA] (FORGE-763) ShellImpl "noInitMode" concept mismanages completers

[Matt Benson (JIRA)]

     [ https://issues.jboss.org/browse/FORGE-763?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Matt Benson updated FORGE-763:
------------------------------

    Description: 
The _concept_ at work in the current code is that a number of completers are set up when the {{ShellImpl#init()}} {{Startup}} observer is called (that number currently happens to be 1--the instance of {{PluginCommandCompleter}} that is also passed to the {{Startup}} observer).  These are added to an {{AggregateCompleter}} and stored as an instance variable of the {{ShellImpl}}.  At the same time, if the shell is determined to be in {{noInitMode}}, these (1) completers are added to a list deemed {{_deferredCompleters}}.  The idea is that in {{noInitMode}} the reader will not be known at {{init}} time.  Unfortunately, there are a number of implementation problems:

* the method that eventually accomplishes setting the completer on the reader, {{_initCompleters()}} is parameterized with a completer, but this completer is never used in the body of the method.  Instead, the completer member is sent.
* the {{_deferredCompleters}} list is only initialized if it is null.  In the test scenario, the result of this is that the list grows by one repeated instance of the same {{PluginCommandCompleter}} for every {{@Test}} method executed in a given unit test class.
* the {{noInitMode}} portion of {{initReadersAndStreams()}} calls {{_initCompleters()}} once per element of {{_deferredCompleters}}; this has two effects:  it sets the completion handler on the reader multiple times (which is only wasteful), and it adds the instance-bound completer multiple times.  As a result, when e.g. {{#promptWithCompleter()}} temporarily removes the shell's completer, there are still {{n}} remaining copies of the same instance hanging around.

If the two identified oddities with the current code are fixed, a new issue arises:  now, the deferred completers are added individually to the reader.  When {{#promptWithCompleter()}} attempts to remove the instance-bound completer from the reader, nothing happens as this is an {{AggregateCompleter}} wrapping the lone deferred completer, which was itself added to the reader.  Thus, any deferment to be done must still be done with the {{AggregateCompleter}}, obviating the deferred completers concept entirely.

  was:
The *concept* at work in the current code is that a number of completers are set up when the {{ShellImpl#init()}} {{Startup}} observer is called (that number currently happens to be 1--the instance of {{PluginCommandCompleter}} that is also passed to the {{Startup}} observer).  These are added to an {{AggregateCompleter}} and stored as an instance variable of the {{ShellImpl}}.  At the same time, if the shell is determined to be in {{noInitMode}}, these (1) completers are added to a list deemed {{_deferredCompleters}}.  The idea is that in {{noInitMode}} the reader will not be known at {{init}} time.  Unfortunately, there are a number of implementation problems:

* the method that eventually accomplishes setting the completer on the reader, {{_initCompleters()}} is parameterized with a completer, but this completer is never used in the body of the method.  Instead, the completer member is sent.
* the {{_deferredCompleters}} list is only initialized if it is null.  In the test scenario, the result of this is that the list grows by one repeated instance of the same {{PluginCommandCompleter}} for every {{@Test}} method executed in a given unit test class.
* the {{noInitMode}} portion of {{initReadersAndStreams()}} calls {{_initCompleters()}} once per element of {{_deferredCompleters}}; this has two effects:  it sets the completion handler on the reader multiple times (which is only wasteful), and it adds the instance-bound completer multiple times.  As a result, when e.g. {{#promptWithCompleter()}} temporarily removes the shell's completer, there are still {{n}} remaining copies of the same instance hanging around.

If the two identified oddities with the current code are fixed, a new issue arises:  now, the deferred completers are added individually to the reader.  When {{#promptWithCompleter()}} attempts to remove the instance-bound completer from the reader, nothing happens as this is an {{AggregateCompleter}} wrapping the lone deferred completer, which was itself added to the reader.  Thus, any deferment to be done must still be done with the {{AggregateCompleter}}, obviating the deferred completers concept entirely.


    
> ShellImpl "noInitMode" concept mismanages completers
> ----------------------------------------------------
>
>                 Key: FORGE-763
>                 URL: https://issues.jboss.org/browse/FORGE-763
>             Project: Forge
>          Issue Type: Bug
>          Components: Shell
>    Affects Versions: 1.2.0.Final
>            Reporter: Matt Benson
>            Assignee: Matt Benson
>
> The _concept_ at work in the current code is that a number of completers are set up when the {{ShellImpl#init()}} {{Startup}} observer is called (that number currently happens to be 1--the instance of {{PluginCommandCompleter}} that is also passed to the {{Startup}} observer).  These are added to an {{AggregateCompleter}} and stored as an instance variable of the {{ShellImpl}}.  At the same time, if the shell is determined to be in {{noInitMode}}, these (1) completers are added to a list deemed {{_deferredCompleters}}.  The idea is that in {{noInitMode}} the reader will not be known at {{init}} time.  Unfortunately, there are a number of implementation problems:
> * the method that eventually accomplishes setting the completer on the reader, {{_initCompleters()}} is parameterized with a completer, but this completer is never used in the body of the method.  Instead, the completer member is sent.
> * the {{_deferredCompleters}} list is only initialized if it is null.  In the test scenario, the result of this is that the list grows by one repeated instance of the same {{PluginCommandCompleter}} for every {{@Test}} method executed in a given unit test class.
> * the {{noInitMode}} portion of {{initReadersAndStreams()}} calls {{_initCompleters()}} once per element of {{_deferredCompleters}}; this has two effects:  it sets the completion handler on the reader multiple times (which is only wasteful), and it adds the instance-bound completer multiple times.  As a result, when e.g. {{#promptWithCompleter()}} temporarily removes the shell's completer, there are still {{n}} remaining copies of the same instance hanging around.
> If the two identified oddities with the current code are fixed, a new issue arises:  now, the deferred completers are added individually to the reader.  When {{#promptWithCompleter()}} attempts to remove the instance-bound completer from the reader, nothing happens as this is an {{AggregateCompleter}} wrapping the lone deferred completer, which was itself added to the reader.  Thus, any deferment to be done must still be done with the {{AggregateCompleter}}, obviating the deferred completers concept entirely.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira