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