RFR: 5904: Merge org.openjdk.jmc.ui.celleditors with org.openjdk.jmc.rjmx.ui.celleditors [v4]

[Alex Macdonald]

On Thu, 19 Aug 2021 20:45:57 GMT, Jie Kang <jkang at openjdk.org> wrote:

>> I took another look into this, and I'm not quite sure where it makes sense to fix this.
>> 
>> This dependency happened because `FilterEditor` (currently in jmc.ui.misc) requires celleditors. `FilterEditor` is also only used in flightrecorder.ui.
>> 
>> rjmx.ui has a dependency on jmc.ui. If celleditors moves into rjmx.ui like this and the issue suggests, then FitlerEditor needs to move because it would rely on a cyclic dependency back onto rjmx.ui.
>> 
>> In this case, to have no introduced dependencies `FilterEditor` would need to move to a package that both imports rjmx.ui, and is imported by flightrecorder.ui.. which isn't any I believe. FilterEditor could be moved into something like jmc.console and add that dependency on flightrecorder.ui, but I'm not sure there's a happy path here.
>> 
>> In a different approach, celleditors could be consolidated into jmc.ui.celleditors instead and keep FilterEditor in jmc.ui.misc. This would require a rjmx.subscription.internal rjmx.services be made available to jmc.ui.celleditors, and a jmc.rjmx dependency added onto jmc.ui. I have a branch here that gives that a try: https://github.com/aptmac/jmc/commit/90c54f202ab19ad2dbf4fdc198feaf6dd1e6d2d3
>
> Would you be able to do a quick text diagram of the dependency chain? Like 
> 
> a->b->c
> d->c
> ...
> 
> 
> Is there a parent `org.openjdk.jmc.ui` where any shared classes for `rjmx.ui` and `flightrecorder.ui` can go?

Sure I'll give this a try.

So what I'm having trouble with here is that `FilterEditor` ultimately needs to be used by `flightrecorder.ui.common`

FilterEditor requires celleditors, which in HEAD is split between `org.openjdk.jmc.ui.celleditors` and `org.openjdk.jmc.rjmx.ui`. The only celleditors class that FilterEditor needs is CommonCellEditors which is in jmc.ui, so there's no dependency on rjmx.ui. rjmx.ui also has a dependency on jmc.ui.

So we have:

jmc.ui.celleditors.celleditors->jmc.ui.misc.FilterEditor->flightrecorder.ui.common
jmc.ui -> jmc.rjmx.ui


If we're consolidating celleditors into rjmx.ui, then we have:

jmc.rjmx.ui -> jmc.ui.misc.FilterEditor -> flightrecorder.ui.common
jmc.ui -> jmc.rjmx.ui


In this case, we can break the cycle by moving FilterEditor elsewhere else. In the approach here I placed it in flightrecorder.ui because that's ultimately where it gets used anyways. But either way I guess it'd go rjmx.ui.celleditors -> "something" -> flightrecorder.ui, so it's a matter of being a direct dependency or not.

If instead we consolidate celleditors into jmc.ui.celleditors, then we have:

jmc.ui -> jmc.rjmx.ui
jmc.rjmx.services + jmc.rjmx.subscription.internal -> jmc.ui.misc.FilterEditor -> flightrecorder.ui.common

because jmc.ui.celleditors would need the apporpriate files that rjmx.ui had access to from rjmx.

-------------

PR: https://git.openjdk.java.net/jmc/pull/271