On Wed, Jun 10, 2026 at 7:26 PM Mark Thomas <[email protected]> wrote:
>
> On 10/06/2026 15:43, [email protected] wrote:
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > rmaucher pushed a commit to branch main
> > in repository https://gitbox.apache.org/repos/asf/tomcat.git
> >
> >
> > The following commit(s) were added to refs/heads/main by this push:
> >       new 4d20e86db3 Minor fixes from code review
> > 4d20e86db3 is described below
> >
> > commit 4d20e86db36bbab6148a4704a69914eaaa9fae64
> > Author: remm <[email protected]>
> > AuthorDate: Wed Jun 10 16:42:47 2026 +0200
> >
> >      Minor fixes from code review
>
> <snip/>
>
> > diff --git a/java/org/apache/catalina/ant/jmx/JMXAccessorConditionBase.java 
> > b/java/org/apache/catalina/ant/jmx/JMXAccessorConditionBase.java
> > index e114f25439..7f64bbc81b 100644
> > --- a/java/org/apache/catalina/ant/jmx/JMXAccessorConditionBase.java
> > +++ b/java/org/apache/catalina/ant/jmx/JMXAccessorConditionBase.java
> > @@ -22,6 +22,7 @@ import java.net.MalformedURLException;
> >   import javax.management.MBeanServerConnection;
> >   import javax.management.ObjectName;
> >
> > +import org.apache.tools.ant.BuildException;
> >   import org.apache.tools.ant.ProjectComponent;
> >   import org.apache.tools.ant.taskdefs.condition.Condition;
> >
> > @@ -233,7 +234,7 @@ public abstract class JMXAccessorConditionBase extends 
> > ProjectComponent implemen
> >                   return result.toString();
> >               }
> >           } catch (Exception e) {
> > -            // ignore access or connection open errors
> > +            throw new BuildException("Cannot access JMX value for 
> > condition", e);
> >           }
> >           return null;
> >       }
>
> What was the reaosn for this change? It breaks the use of Ant's
> <waitFor> task (documented in the class level JavaDoc).
>
> I was going to remove the exception and add an explanatory comment but I
> wanted to check on the reason for the change first.

Since everything gets swallowed, it was harder to see where things
went wrong. Ok for reverting if there's a reason.

> <snip/>
>
> > diff --git a/java/org/apache/catalina/ant/jmx/JMXAccessorTask.java 
> > b/java/org/apache/catalina/ant/jmx/JMXAccessorTask.java
> > index 8e27366a44..d9d2d1cd67 100644
> > --- a/java/org/apache/catalina/ant/jmx/JMXAccessorTask.java
> > +++ b/java/org/apache/catalina/ant/jmx/JMXAccessorTask.java
>
> <snip/>
>
> > @@ -562,7 +560,7 @@ public class JMXAccessorTask extends 
> > BaseRedirectorHelperTask {
> >       }
> >
> >       /**
> > -     * Convert string to datatype FIXME How we can transfer values from 
> > ant project reference store (ref)?
> > +     * Convert string to datatype.
> >        *
> >        * @param value     The value
> >        * @param valueType The type
> > @@ -686,15 +684,16 @@ public class JMXAccessorTask extends 
> > BaseRedirectorHelperTask {
> >                   }
> >               }
> >           } else if (result instanceof TabularDataSupport data) {
> > -            for (Object key : data.keySet()) {
> > -                for (Object key1 : ((List<?>) key)) {
> > -                    CompositeData valuedata = data.get(new Object[] { key1 
> > });
> > -                    Object value = valuedata.get("value");
> > -                    OpenType<?> type = 
> > valuedata.getCompositeType().getType("value");
> > -                    if (type instanceof SimpleType<?>) {
> > -                        setProperty(propertyPrefix + "." + key1, value);
> > +            for (Object rowObj : data.values()) {
> > +                CompositeData row = (CompositeData) rowObj;
> > +                CompositeType rowType = row.getCompositeType();
> > +                for (String fieldName : rowType.keySet()) {
> > +                    Object fieldValue = row.get(fieldName);
> > +                    OpenType<?> fieldType = rowType.getType(fieldName);
> > +                    if (fieldType instanceof SimpleType<?>) {
> > +                        setProperty(propertyPrefix + "." + fieldName, 
> > fieldValue);
> >                       } else {
> > -                        createProperty(propertyPrefix + "." + key1, value);
> > +                        createProperty(propertyPrefix + "." + fieldName, 
> > fieldValue);
> >                       }
> >                   }
> >               }
>
> The new code only writes the first row of the TabularDate to Ant
> properties. Subsequent rows are ignored. I suspect that wasn't intended.
>
> Do we want to add some sort of numerical index and length (like arrays)?

This is a complex one (to me), and I didn't understand how it worked.
The suggestion to go for the values instead looked nicer. It looks
more correct now so I'll revert it as well.
The typing is really really messy obviously in these old JMX classes.

Rémy

> <snip/>
>
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to