Skip to content

Commit c016d8f

Browse files
AXIS2-5862 Document misleading currentPhaseIndex / currentHandlerIndex naming
The two index fields on MessageContext are named opposite to what the names imply, and that confusion is what led the AXIS2-5862 reporter to propose a patch that would have regressed the engine: * currentHandlerIndex indexes the MessageContext.executionChain, whose elements are Phase objects (Phase implements Handler). So it is effectively "index of the current Phase within the execution chain." AxisEngine.invoke walks the chain via get/setCurrentHandlerIndex. * currentPhaseIndex indexes the handlers list INSIDE the Phase that is currently executing. So it is effectively "index of the current Handler within the current Phase." Phase.invoke and Phase.flowComplete walk that list via get/setCurrentPhaseIndex. Renaming the fields would be a semver-major break (public getters/ setters, Externalizable wire format, downstream module code, etc.), so the fields keep their historical names. This commit adds the explanation directly to the fields' Javadoc, the four accessor methods' Javadoc, and the confusing local-variable reassignment in Phase.flowComplete where we read getCurrentPhaseIndex() into a local called currentHandlerIndex. No functional change.
1 parent c605835 commit c016d8f

2 files changed

Lines changed: 50 additions & 3 deletions

File tree

modules/kernel/src/org/apache/axis2/context/MessageContext.java

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,12 +346,29 @@ public static void setCurrentMessageContext(MessageContext ctx) {
346346
private transient ConfigurationContext configurationContext;
347347

348348
/**
349-
* @serial Index into the executuion chain of the currently executing handler
349+
* @serial Index into the execution chain of the currently executing handler.
350+
* <p>
351+
* NOTE on naming (see AXIS2-5862 for history): the execution chain stored
352+
* on this MessageContext is a {@code List<Handler>} whose elements are
353+
* actually {@link org.apache.axis2.engine.Phase} objects (Phase implements
354+
* Handler). So despite the field name, {@code currentHandlerIndex} is
355+
* effectively "index of the current Phase within the execution chain."
356+
* {@link org.apache.axis2.engine.AxisEngine#invoke} walks the chain using
357+
* this field and {@link #setCurrentHandlerIndex(int)}.
350358
*/
351359
private int currentHandlerIndex;
352360

353361
/**
354-
* @serial Index into the current Phase of the currently executing handler (if any)
362+
* @serial Index into the current Phase of the currently executing handler (if any).
363+
* <p>
364+
* NOTE on naming (see AXIS2-5862 for history): despite the field name,
365+
* {@code currentPhaseIndex} does NOT index the chain of phases --
366+
* {@link #currentHandlerIndex} does that. This field indexes the
367+
* {@code handlers} list INSIDE the Phase that is currently executing,
368+
* so it is effectively "index of the current Handler within the current
369+
* Phase." {@link org.apache.axis2.engine.Phase#invoke} and
370+
* {@link org.apache.axis2.engine.Phase#flowComplete} use this field
371+
* via {@link #getCurrentPhaseIndex()} / {@link #setCurrentPhaseIndex(int)}.
355372
*/
356373
private int currentPhaseIndex;
357374

@@ -625,10 +642,21 @@ public ConfigurationContext getConfigurationContext() {
625642
return configurationContext;
626643
}
627644

645+
/**
646+
* @return The index of the current Phase within the execution chain.
647+
* See the note on {@link #currentHandlerIndex} for why the
648+
* field is named the way it is.
649+
*/
628650
public int getCurrentHandlerIndex() {
629651
return currentHandlerIndex;
630652
}
631653

654+
/**
655+
* @return The index of the currently executing Handler within the
656+
* current Phase's handlers list. See the note on
657+
* {@link #currentPhaseIndex} for why the field is named the
658+
* way it is.
659+
*/
632660
public int getCurrentPhaseIndex() {
633661
return currentPhaseIndex;
634662
}
@@ -1272,10 +1300,21 @@ public void setConfigurationContext(ConfigurationContext context) {
12721300
configurationContext = context;
12731301
}
12741302

1303+
/**
1304+
* Sets the index of the current Phase within the execution chain.
1305+
* See the note on {@link #currentHandlerIndex} for why the field
1306+
* is named the way it is.
1307+
*/
12751308
public void setCurrentHandlerIndex(int currentHandlerIndex) {
12761309
this.currentHandlerIndex = currentHandlerIndex;
12771310
}
12781311

1312+
/**
1313+
* Sets the index of the currently executing Handler within the
1314+
* current Phase's handlers list. See the note on
1315+
* {@link #currentPhaseIndex} for why the field is named the way
1316+
* it is.
1317+
*/
12791318
public void setCurrentPhaseIndex(int currentPhaseIndex) {
12801319
this.currentPhaseIndex = currentPhaseIndex;
12811320
}

modules/kernel/src/org/apache/axis2/engine/Phase.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,15 @@ public void flowComplete(MessageContext msgContext) {
342342
}
343343

344344
// This will be non-zero if we failed during execution of one of the
345-
// handlers in this phase
345+
// handlers in this phase.
346+
//
347+
// Naming note: the local is called "currentHandlerIndex" because that
348+
// is what the value *semantically* is -- the index of the handler
349+
// within this Phase at which the fault was raised. The getter is
350+
// called getCurrentPhaseIndex() because that is the MessageContext
351+
// field name, but despite the name the field holds a handler-within-
352+
// phase index, not a phase-within-chain index. See the Javadoc on
353+
// MessageContext.currentPhaseIndex and AXIS2-5862.
346354
int currentHandlerIndex = msgContext.getCurrentPhaseIndex();
347355
if (currentHandlerIndex == 0) {
348356
currentHandlerIndex = handlers.size();

0 commit comments

Comments
 (0)