Skip to content

Conversation

@kusalk
Copy link
Member

@kusalk kusalk commented Jun 18, 2024

@kusalk kusalk force-pushed the WW-5429-param-anno-log branch 3 times, most recently from f1ddc88 to 35ca03c Compare June 18, 2024 09:27
@kusalk kusalk force-pushed the WW-5429-param-anno-log branch from 35ca03c to b96cf2c Compare June 18, 2024 09:36
* @return <code>(hasActionErrors() || hasFieldErrors())</code>
*/
boolean hasErrors();
default boolean hasErrors() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added default implementation which matches the JavaDoc, makes implementing this class simpler

}

private void appenExpression(String expr) {
private void appendExpression(String expr) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed typo

*/
public class DebugUtils {

public static void notifyDeveloperOfError(Logger log, Object action, String message) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted this method out of ParametersInterceptor for reuse


// then
assertEquals(3, action.getActionMessages().size());
assertEquals(3, action.getActionErrors().size());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Action errors instead of Action messages to communicate developer errors (only impacts DevMode)

String msg2 = actionErrors.get(1);
String msg3 = actionErrors.get(2);

assertEquals("Unexpected Exception caught setting 'expression' on 'class org.apache.struts2.interceptor.parameter.ValidateAction: Error setting expression 'expression' with value '#f=#_memberAccess.getClass().getDeclaredField('allowStaticMethodAccess'),#f.setAccessible(true),#f.set(#_memberAccess,true),#req=@org.apache.struts2.ServletActionContext@getRequest(),#resp=@org.apache.struts2.ServletActionContext@getResponse().getWriter(),#resp.println(#req.getRealPath('/')),#resp.close()'", msg1);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These messages now include both the context message as well as the exception message. Whilst they are very similar in this test example, it's not guaranteed to be the case

@kusalk kusalk requested a review from lukaszlenart June 18, 2024 09:42
* in a Collection. Field level error messages are kept in a Map from String field name to a List of field error msgs.
*
* @author plightbo
* @author plightbo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove the @author tag next time (recommended by ASF)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done :)

/**
* @since 6.5.0
*/
public class DebugUtils {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be final or maybe make an interface which can be mixed in?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made it final, not sure it makes sense as a mix-in interface

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
65.8% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@kusalk kusalk merged commit 898a8d9 into master Jun 21, 2024
@kusalk kusalk deleted the WW-5429-param-anno-log branch June 21, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants