From 29182f97198b37e75d57a85bd5e8e4dce90ad7b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Javier=20Cavero=20L=C3=B3pez?= Date: Thu, 30 Jul 2026 18:46:38 +0200 Subject: [PATCH] Accept an expected consumption of zero Two falsy checks treated a numeric zero as an absent value, so a caller who named a limit and declared it costs nothing was told they had not named it at all. The same check on the sum also refused an ordinary positive consumption whenever the usage level was still zero, which is the state of every contract on its first call and after every renewal. --- .../feature-evaluation/featureEvaluation.ts | 7 +- ...eature-evaluation.zero-consumption.test.ts | 112 ++++++++++++++++++ 2 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 api/src/test/feature-evaluation.zero-consumption.test.ts diff --git a/api/src/main/utils/feature-evaluation/featureEvaluation.ts b/api/src/main/utils/feature-evaluation/featureEvaluation.ts index fd0fd9e..4532307 100644 --- a/api/src/main/utils/feature-evaluation/featureEvaluation.ts +++ b/api/src/main/utils/feature-evaluation/featureEvaluation.ts @@ -226,7 +226,7 @@ function _buildSuccessResult( subscriptionContext[limitKey], expectedConsumption[limitKey] ); - if (!updatedUsageLevel) { + if (updatedUsageLevel === undefined) { return _createErrorResult( 'INVALID_EXPECTED_CONSUMPTION', `No expectedConsumption value was provided for limit '${limitKey}', which is used in the evaluation of feature '${featureId}'. Please note that if you provide an expectedConsumption for any limit, you must provide it for all limits involved in that feature's evaluation.` @@ -250,7 +250,10 @@ function _updateUsageLevel( currentUsageLevel: number, expectedConsumption?: number ): number | undefined { - if (!expectedConsumption) { + // `undefined` means the caller did not mention this limit, which is the + // error the caller is told about. `0` means they mentioned it and it costs + // nothing - a different statement, and one a falsy check cannot tell apart. + if (expectedConsumption === undefined || expectedConsumption === null) { return undefined; } diff --git a/api/src/test/feature-evaluation.zero-consumption.test.ts b/api/src/test/feature-evaluation.zero-consumption.test.ts new file mode 100644 index 0000000..153205f --- /dev/null +++ b/api/src/test/feature-evaluation.zero-consumption.test.ts @@ -0,0 +1,112 @@ +import { describe, it, expect } from 'vitest'; +import { evaluateFeature } from '../main/utils/feature-evaluation/featureEvaluation'; +import type { + EvaluationContext, + FeatureEvaluationResult, + PricingContext, + SubscriptionContext, +} from '../main/types/models/FeatureEvaluation'; + +/** + * An expected consumption of zero. + * + * A caller who provides `expectedConsumption` must provide it for every limit + * involved in the feature's evaluation, or be refused. So the only way to say + * "this limit takes part in the evaluation but this call does not spend it" is + * to pass zero - which two falsy checks rejected as if the limit had been left + * out altogether. + * + * The second of those checks also refused a perfectly ordinary positive + * consumption, whenever the current usage level happened to be zero: a brand + * new contract, or the first call of a renewal period. + */ + +const FEATURE = 'petclinic-pets'; +const LIMIT = 'petclinic-maxPets'; + +const EXPRESSION = `subscriptionContext['${LIMIT}'] < pricingContext['usageLimits']['${LIMIT}']`; + +const pricingContext: PricingContext = { + features: { [FEATURE]: true }, + usageLimits: { [LIMIT]: 10 }, +}; + +const evaluationContext: EvaluationContext = { [FEATURE]: EXPRESSION }; + +/** @param usageLevel what the contract has consumed so far. */ +async function evaluate(usageLevel: number, expectedConsumption?: Record) { + const subscriptionContext: SubscriptionContext = { [LIMIT]: usageLevel }; + + return (await evaluateFeature(FEATURE, pricingContext, subscriptionContext, evaluationContext, { + simple: false, + expectedConsumption, + // No userId, so nothing is written: this is about the verdict, not the + // bookkeeping that follows it. + })) as FeatureEvaluationResult; +} + +describe('expectedConsumption of zero', () => { + it('is accepted, and leaves the usage level where it was', async () => { + const result = await evaluate(5, { [LIMIT]: 0 }); + + expect(result.error).toBeNull(); + expect(result.eval).toBe(true); + expect(result.used).toEqual({ [LIMIT]: 5 }); + }); + + it('is accepted on a contract that has consumed nothing yet', async () => { + // Both zeroes at once: the usage level and the consumption. This is the + // case a `!updatedUsageLevel` check gets wrong even after `0 + 0` has been + // computed correctly. + const result = await evaluate(0, { [LIMIT]: 0 }); + + expect(result.error).toBeNull(); + expect(result.used).toEqual({ [LIMIT]: 0 }); + }); + + it('is not reported as a missing value', async () => { + const result = await evaluate(0, { [LIMIT]: 0 }); + + expect(result.error?.code).not.toBe('INVALID_EXPECTED_CONSUMPTION'); + }); +}); + +describe('expectedConsumption on an untouched usage level', () => { + it('adds to a usage level of zero', async () => { + // Not about zero consumption at all: a plain consumption of 1 on a brand + // new contract. `1` is truthy, but only because the addition happens to + // leave a truthy total. + const result = await evaluate(0, { [LIMIT]: 1 }); + + expect(result.error).toBeNull(); + expect(result.used).toEqual({ [LIMIT]: 1 }); + }); + + it('adds to a non-zero usage level, as before', async () => { + const result = await evaluate(5, { [LIMIT]: 3 }); + + expect(result.used).toEqual({ [LIMIT]: 8 }); + }); +}); + +describe('expectedConsumption that really is missing', () => { + it('is still refused when the limit is left out', async () => { + const result = await evaluate(5, { 'petclinic-someOtherLimit': 1 }); + + expect(result.error?.code).toBe('INVALID_EXPECTED_CONSUMPTION'); + }); + + it('reports the current usage level when no consumption is given at all', async () => { + const result = await evaluate(5, undefined); + + expect(result.error).toBeNull(); + expect(result.used).toEqual({ [LIMIT]: 5 }); + }); + + it('treats an empty object as no consumption', async () => { + const result = await evaluate(5, {}); + + expect(result.error).toBeNull(); + expect(result.used).toEqual({ [LIMIT]: 5 }); + }); +});