fix for MeasurementScheduleManagerBean.update

Ian Springer ian.springer at redhat.com
Wed Nov 3 19:55:19 UTC 2010


I reviews all the related methods again, and the logic looks correct to 
me. I added some additional comments and javadoc to clarify the 
semantics of the collection interval parameter.

On 11/02/2010 09:59 AM, Ian Springer wrote:
>    Sure, will look into those changes. Thanks for reviewing.
>
> On 11/02/2010 12:56 AM, Joseph Marques wrote:
>>     Ian, interesting fix.
>>
>> Looks like the enhancement for
>> 'modifyDefaultCollectionIntervalForMeasurementDefinitions(4 args)' to
>> support the amended JDoc was relatively straightforward.  However, I
>> think the logic:
>>
>> -        boolean enableDisable = (collectionInterval>    0);
>> +        boolean enable = (collectionInterval>= 0);
>>
>> Should move into
>> 'modifyDefaultCollectionIntervalForMeasurementDefinitions(5 args)' which
>> implements the actual update logic.  This will allow existing and future
>> call paths to have the same post-condition.
>>
>> Looking deeper into
>> 'modifyDefaultCollectionIntervalForMeasurementDefinitions(5 args)',  I
>> notice that we don't do any validation on the 'collectionInterval'
>> parameter.  When updating measurement schedules we make sure to call the
>> 'verifyMinimumCollectionInterval' method, but we don't seem to prevent
>> users from setting default collection intervals to invalid values.
>>
>> Can you add that validation?
>>
>> On 10/29/2010 07:44 PM, ips wrote:
>> ...
>>>     modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/measurement/MeasurementScheduleManagerBean.java                 |   15 +-
>>>     7 files changed, 143 insertions(+), 36 deletions(-)
>>>
>>> New commits:
>>> commit 0a0aa07d0e4cdf71b1dee4a7631bd70e50715072
>>> Author: Ian Springer<ian.springer at redhat.com>
>>> Date:   Fri Oct 29 19:43:38 2010 -0400
>>>
>>>        add support to metric template view for specifying whether existing schedules should be updated
>> ...
>>> diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/measurement/MeasurementScheduleManagerBean.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/measurement/MeasurementScheduleManagerBean.java
>>> index ccd65bc..3dc8b05 100644
>>> --- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/measurement/MeasurementScheduleManagerBean.java
>>> +++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/measurement/MeasurementScheduleManagerBean.java
>>> @@ -332,13 +332,16 @@ public class MeasurementScheduleManagerBean implements MeasurementScheduleManage
>>>         }
>> ...
>>> +    * @param collectionInterval       if>    0, enable the metric with this value as the the new collection
>>> +    *                                 interval, in milliseconds; if == 0, enable the metric with its current
>>> +    *                                 collection interval; if<    0, disable the metric
>>>         * @param updateExistingSchedules  If true, then existing schedules for this definition will also be updated.
>>>         */
>>>         @RequiredPermission(Permission.MANAGE_SETTINGS)
>>> @@ -358,13 +361,13 @@ public class MeasurementScheduleManagerBean implements MeasurementScheduleManage
>>>                 return;
>>>             }
>>>
>>> -        boolean enableDisable = (collectionInterval>    0);
>>> +        boolean enable = (collectionInterval>= 0);
>>>
>>>             // batch the modifications to prevent the ORA error about IN clauses containing more than 1000 items
>>>             for (int batchIndex = 0; (batchIndex<    measurementDefinitionIds.length); batchIndex += 1000) {
>>>                 int[] batchIdArray = ArrayUtils.copyOfRange(measurementDefinitionIds, batchIndex, batchIndex + 1000);
>>>
>>> -            modifyDefaultCollectionIntervalForMeasurementDefinitions(subject, batchIdArray, enableDisable,
>>> +            modifyDefaultCollectionIntervalForMeasurementDefinitions(subject, batchIdArray, enable,
>>>                     collectionInterval, updateExistingSchedules);
>>>             }
>>>         }
>>>
>>>
>>> _______________________________________________
>>> rhq-commits mailing list
>>> rhq-commits at lists.fedorahosted.org
>>> https://fedorahosted.org/mailman/listinfo/rhq-commits
>> _______________________________________________
>> rhq-devel mailing list
>> rhq-devel at lists.fedorahosted.org
>> https://fedorahosted.org/mailman/listinfo/rhq-devel
>


-- 
Ian Springer
Principal Software Developer
JBoss Operations Network
Red Hat
ian.springer at redhat.com



More information about the rhq-devel mailing list