[jira] [Commented] (CASSANDRA-17772) Improve documentation around query methods in CQLTester and GuardrailTester

["Josh McKenzie (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CASSANDRA-17772?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17570143#comment-17570143 ] 

Josh McKenzie commented on CASSANDRA-17772:
-------------------------------------------

[Diff here|https://github.com/apache/cassandra/compare/trunk...josh-mckenzie:cassandra:CASSANDRA-17772?expand=1]

No CI needed as this is purely javadoc in {{GuardrailTester}} and {{CQLTester}}

[~adelapena] - any chance you have a few minutes to spare to review this?

> Improve documentation around query methods in CQLTester and GuardrailTester
> ---------------------------------------------------------------------------
>
>                 Key: CASSANDRA-17772
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-17772
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Feature/Guardrails
>            Reporter: Josh McKenzie
>            Assignee: Josh McKenzie
>            Priority: Normal
>
> Anything that relies on CQLTester.executeFormattedQuery (the assertInvalidThrowMessage methods for instance) will use internal client state rather than the client state specified for the query, thus nullifying any guardrail or systems keyspace permission checks as the ClientState.isInternal flag overrides all such permission checking. Reference: [link|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/service/ClientState.java#L385-L388]
>  
> See chain of CQLTester -> ClientState.isInternal here if interested:
> [CQLTester|https://github.com/apache/cassandra/blob/trunk/test/unit/org/apache/cassandra/cql3/CQLTester.java#L1325]
> [QueryProcessor|https://github.com/apache/cassandra/blob/8451acc8d8dcfee20d692d1e70ae11b60d2f004e/src/java/org/apache/cassandra/cql3/QueryProcessor.java#L447]
>  
> This is pretty easy to stumble across when testing guardrails as GuardrailTester adds a variety of assertFails and assertThrows signatures that _do_ respect the client state and thus apply guardrails ([example|https://github.com/apache/cassandra/blob/trunk/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java#L271])
>  
> We should add documentation to the method calls in CQLTester and GuardrailTester to reflect this discrepancy as it can easily trip someone up writing tests for guardrails.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org