Uh oh!
There was an error while loading. Please reload this page.
Fix two bugs in RPCIndividual - #1683
Open
LautaroPetaccio wants to merge 1 commit into
Open
Conversation
Both were found while writing an equivalent individual for another problem type and copying this one's shape. seeIndexedRPCCalls always returned an empty map. It asks for children of type RPCCallAction, but the calls are not children of the individual directly: each is wrapped in an EnterpriseActionGroup alongside the external-service actions belonging to it, so nothing ever matches. The group is now unwrapped. The method has no callers today, which is presumably why this went unnoticed. The convenience constructor threw IndexOutOfBoundsException whenever it was used without external-service actions. externalServicesActions defaults to an empty list, and while the assert on its size is guarded by isNotEmpty, the indexing that follows is not, so externalServicesActions[index] fails for the very first call. Only callers that pass a list of matching size worked, which is what the existing test helper does. Adds RPCIndividualTest, which the class had none of.
arcuri82
commented
Aug 17, 2026
Collaborator
hi @LautaroPetaccio thx! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two bugs in
RPCIndividual, both found while writing an equivalent individual for another problem type and copying this one's shape. Independent of that work, so sent on its own.seeIndexedRPCCallsalways returns an empty mapgetIndexedChildrenfilters the individual's direct children by type, but the calls are not direct children: the constructor wraps each in anEnterpriseActionGroup, alongside the external-service actions belonging to it. So nothing ever matches the filter and the result is always empty.Compare
RestIndividual.getIndexedResourceCalls, which works becauseRestResourceCallsreally is the child type there.The method has no callers today, which is presumably why it went unnoticed. The fix unwraps the group.
The convenience constructor throws whenever it is used without external services
addAll(actions.mapIndexed { index, rpcCallAction ->if (externalServicesActions.isNotEmpty()){ Lazy.assert { actions.size == externalServicesActions.size } } EnterpriseActionGroup(...).apply { addChildrenToGroup(externalServicesActions[index], ...) // <- not guarded }})externalServicesActionsdefaults to an empty list. TheisNotEmpty()check guards only the assert; the indexing right after it is unguarded, soexternalServicesActions[index]throwsIndexOutOfBoundsExceptionon the very first call.In practice only callers passing a list of matching size work — which is what
EvaluatedIndividualBuilder.buildEvaluatedRPCIndividualdoes, so the existing tests never hit it. Constructing anRPCIndividualthe way the default parameters invite you to fails immediately.The fix adds the missing bound check, leaving behaviour unchanged when a list is supplied.
Testing
Adds
RPCIndividualTest, which the class had none of — four tests covering the indexed lookup, the indices actually addressing the children that hold the calls, calls found past initializing actions, and the empty case. The first of them fails on both bugs before the fix.RPCActionNamingStrategyTest,TestSuiteWriterRPCTestandTestSuiteOrganizerTeststill pass.