|
1
|
*Modified areas:* com.goldencode.p2j.main (8 files: @Broker@, @BrokerCore@, @BrokerDiscovery@ [new], @BrokerManager@, @BrokerServerServices@, @ClientsToPortsGenerator@, @HostsManager@, @WebClientsManager@), com.goldencode.p2j.security (3 files: @ProcessAccount@, @SecurityCache@, @SecurityManager@), com.goldencode.p2j.util (2 files: @BaseConfigItem@ [new], @ConfigItem@), src/dir_schema.xml, tools/docker + tools/scripts (5 files)
|
|
2
|
|
|
3
|
*Domains reviewed:* fwd-server-infrastructure, fwd-runtime-core-datatypes
|
|
4
|
|
|
5
|
h2. Findings
|
|
6
|
|
|
7
|
* *[MAJOR]* _style_ @BrokerManager@.@registerBroker@: The @@param@ tags use 4-space alignment (column 10) while the @@return@/@@throws@ tags in the same Javadoc block still use the old 2-space alignment (column 8/9) — internal inconsistency within one comment block.
|
|
8
|
* *[MAJOR]* _style_ @HostsManager@.@HostData@: Javadoc on @isAgentsPortsRangeConfigured@/@isPortsRestricted@/@isJavaDebuggerNeeded@/@isJmxAgentNeeded@ uses non-standard @@true@@/@@false@@ markup instead of the codebase-standard @{@code true}@/@{@code false}@ (used in 644 other files); this is the only file using this markup.
|
|
9
|
* *[MAJOR]* _style_ @WebClientsManager@.@createResourceQueue@: After removing the @new LinkedList<>(...)@ wrapper around the @IntStream@ chain, the @.boxed()@/@.collect(...)@ continuation lines were left at their old indentation (dot at column 55) instead of realigning under the now-shorter @IntStream.rangeClosed(...)@ (dot at column 38), leaving a broken gap.
|
|
10
|
* *[MAJOR]* _style_ @WebClientsManager@.@allocateClient@: The condition @if (!(hostData.isPortsRestricted() || hostData.isAgentsPortsRangeConfigured()))@ places @||@ at the start of the continuation line instead of trailing the prior line, contrary to the multi-line-condition operator-placement rule.
|
|
11
|
* *[MAJOR]* _style_ @BaseConfigItem@ (file header Abstract line): 114 characters, exceeding the 110-char limit; wrap one word earlier.
|
|
12
|
* *[MAJOR]* _style_ @BaseConfigItem@ (first constructor): Wrapped parameters use a flat 9-space indent instead of aligning to the opening parenthesis, while the next constructor in the same new file wraps correctly aligned — internally inconsistent within one new file.
|
|
13
|
* *[MAJOR]* _style_ @ConfigItem@ (constant declarations @CLASSPATH@, @LIB_PATH@, @SPAWNER@, @MAX_RESPONSE_HEADER_SIZE@, @MAX_REQUEST_HEADER_SIZE@, @JVM_ARGS@, @MIN_AGENT_PORT@, @MAX_AGENT_PORT@, etc.): Three different line-wrap conventions applied to the identical declaration shape within the same reformatting pass. Standardize on one wrap style.
|
|
14
|
* *[MINOR]* _functional_ @HostsManager@.@addHost@ / @BrokerManager@.@registerBroker@: A broker co-located with the FWD server (broker node @host@ resolving to loopback, @localhost@/@127.0.0.1@) is mis-indexed and gets the wrong resource pool. The @HostsManager@ constructor pins @hostsMap["localhost"]@ and @["127.0.0.1"]@ to index 1, and @WebClientsManager@ upgrades the @"localhost"@ entry in place with @HostData@ built from the server's own global @webClient/portsRange@. When such a broker registers with @dedicatedHost="localhost"@, @addHost@'s first-registration-wins guard (returns early when @existing.getHostData() != null@) keeps the index-1 seed and discards the broker's own @createResourceQueue@ range, so @generateWithBrokerRanges@ emits @client-2-*@ (broker index ≥2) while @allocateClient@ resolves the host to index 1 (@client-1-*@) — the runtime web-roots have no proxy-map entry and ports come from the server's global range, not the broker's. Trigger: a @brokers@ node whose @host@ is the server loopback on a single-host deployment (@BrokerManager@ does not reject a loopback host).
|
|
15
|
* *[MINOR]* _functional_ @ConfigItem@.@SPAWNER_LAUNCH_TIMEOUT@ (static initializer): The bootstrap option name changed from @remote:spawner:launchTimeout@ to @client:spawnerLaunchTimeout:value@ (verified: old 6-arg ctor → @remote:spawner:launchTimeout@; new @(Class, String, Type, boolean=true)@ ctor → @client:spawnerLaunchTimeout:value@). @BrokerCore.determineLaunchTimeout@ and @BaseClientBuilderOptions@ read it via @category()/group()/key()@ against @BootstrapConfig@, and the item is never written internally — its only source is external bootstrap config with no compatibility alias. Any deployment supplying the timeout under the old @remote.spawner.launchTimeout@ key now silently misses it and falls back to @DEFAULT_TIMEOUT@. Sibling items renamed in the same pass had previously-null option names (dead reads — a genuine fix); this one was live. The rename appears deliberate (BrokerCore history rev 011), but it is a backwards-compat break — confirm it is intended and documented for upgraders.
|
|
16
|
* *[MINOR]* _style_ @Broker@ (fields @brokerAccount@, @host@): Declared without @final@ though each is assigned only once in the constructor — inconsistent with sibling fields @from@/@to@/@index@/@portsRestricted@ added in the same change, which are @final@.
|
|
17
|
* *[MINOR]* _style_ @Broker@ (field javadocs @portsRestricted@/@from@/@to@): Uncapitalized / missing trailing period, unlike adjacent new fields @index@ and @brokerAccount@.
|
|
18
|
* *[MINOR]* _style_ @Broker@ (getters @getFrom@/@getTo@/@getHost@/@getBrokerAccount@ vs @isPortsRestricted@/@getIndex@): @@return@ descriptions inconsistently omit vs include a trailing period within the same block of six getters.
|
|
19
|
* *[MINOR]* _style_ @Broker@ / @BrokerCore@ (@Broker(...)@ ctor, @BrokerCore.start@, @BrokerCore.connect@): New/modified Javadoc uses 4-space @@param@ / 3-space @@return@ column alignment, differing from the same file's untouched methods (3-space/2-space). Normalize.
|
|
20
|
* *[MINOR]* _style_ @BrokerCore@.@initialize@: A blank line sits directly before the method's closing @}@ (after the @agentsPortsRangeConfigured = ...@ call), violating "no blank lines before @}@".
|
|
21
|
* *[MINOR]* _style_ @BrokerCore@.@initialize@: The JMX-agent detection branch has an explanatory comment but the structurally identical java-debugger detection branch above it does not — asymmetric commenting.
|
|
22
|
* *[MINOR]* _style_ @BrokerDiscovery@.@getBroker@: The @portRangeFrom@ and @portRangeTo@ fluent-chain computations are structurally identical but indented differently (12/9/9 vs 16/12/12 spaces).
|
|
23
|
* *[MINOR]* _style_ @BrokerManager@ (between @removeBroker@ and @updateWebClientOptions@): Two consecutive blank lines separate the methods (leftover from a deletion); should be exactly one.
|
|
24
|
* *[MINOR]* _style_ @BrokerManager@.@registerBroker@ / @BrokerServerServices@.@registerBroker@ / @WebClientsManager@.@createResourceQueue@: Trailing whitespace after the comma on the @boolean agentRangeConfigured,@ and @int minAgentPort,@ parameter lines only. Trim.
|
|
25
|
* *[MINOR]* _style_ @ClientsToPortsGenerator@.@generate@: The @from@/@to@ port-range lookups (structurally identical fluent chains) wrap at different points.
|
|
26
|
* *[MINOR]* _style_ @ClientsToPortsGenerator@.@generateWithBrokerRanges@ / @resolvePrefix@: Wrapped @Utils.findDirectoryNodePath(...)@ continuation arguments are misaligned by one column versus the opening @(@.
|
|
27
|
* *[MINOR]* _style_ @HostsManager@.@HostData@ (constructor): Trailing whitespace after the commas on the @agentsPortsRangeConfigured,@ and @minAgentPort,@ parameter lines.
|
|
28
|
* *[MINOR]* _style_ @HostsManager@.@canonicalHostKeys@: Its @LOG.logp(...)@ call packs @Level.WARNING@/method-name/@""@ on one line, whereas every other touched @LOG.logp@ call puts one argument per line.
|
|
29
|
* *[MINOR]* _style_ @HostsManager@ (@HostData@/@HostEntry@ getters): @@return@ descriptions start lowercase, inconsistent with the file's capitalized convention.
|
|
30
|
* *[MINOR]* _style_ @HostsManager@.@HostData@: Blank separator lines between private fields carry trailing whitespace (6 spaces) instead of being empty.
|
|
31
|
* *[MINOR]* _style_ @WebClientsManager@ (@createResourceQueue@, @isAgentRangeConfigured@, constructor @addHost@ call site): Further stray trailing whitespace on some wrapped argument/parameter lines but not siblings.
|
|
32
|
* *[MINOR]* _style_ @WebClientsManager@.@createResourceQueue@ / @isAgentRangeConfigured@: Wrapped parameter declarations mix @String@/@boolean@/@int@ types without aligning parameter names to a common column.
|
|
33
|
* *[MINOR]* _style_ @WebClientsManager@ (@agentsPortsRangeConfigured@ field): Javadoc lacks a trailing period, unlike the adjacent new @LOCAL_HOST_INDEX@ field comment.
|
|
34
|
* *[MINOR]* _style_ @BaseConfigItem@: A whitespace-only line sits directly before the class's closing @}@.
|
|
35
|
* *[MINOR]* _style_ @ConfigItem@.@Type@ (enum): A whitespace-only line sits directly before the enum's closing @}@.
|
|
36
|
* *[MINOR]* _style_ @ConfigItem@ (@BROKER_DEDICATED_MODE@ field javadoc): Continuation lines use two spaces after @*@ instead of one.
|
|
37
|
* *[MINOR]* _style_ @ConfigItem@ (@POLL_TIMEOUT@ field javadoc): Written as a 3-line block comment for a one-sentence description, unlike sibling one-liners.
|
|
38
|
* *[MINOR]* _style_ @ConfigItem@.@createConfigItemOfType@: Declared @public static final <T>@ — redundant @final@ on a static method of an already-@final@ class.
|
|
39
|
* *[MINOR]* _style_ @ConfigItem@ (@CLIENT_PORT_RANGE_TO@ vs @CLIENT_PORT_RANGE_FROM@): Inconsistent trailing whitespace before the line break in otherwise identical adjacent declarations.
|
|
40
|
* *[MINOR]* _style_ dbash.sh (option-parsing @case@ block, @pull="never"@ line): Retains a literal hard tab before its leading spaces rather than being cleaned to 3-space indentation.
|
|
41
|
* *[MINOR]* _style_ docker_build.sh (@publish_image_family@): @if [[ "$push_image" == true ]]; then@ has a double space around @==@.
|
|
42
|
* *[MINOR]* _style_ setup_ncurses6x.sh (@show_usage@): Replacing escaped @\t@ usage strings with a @cat <<EOF@ heredoc introduces literal hard-tab bytes; use aligned spaces instead.
|