From 8152bec2fd885d81394ae110611384d95464143a Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Mon, 17 Jul 2023 12:25:28 -0400 Subject: [PATCH] Allow null as a constant --- lib/flipper/expression.rb | 2 +- lib/flipper/expressions/percentage.rb | 2 +- lib/flipper/gates/expression.rb | 2 +- packages/expressions/examples/Equal.json | 8 ++++++ .../expressions/examples/expressions.json | 5 +++- packages/expressions/lib/constant.js | 2 +- packages/expressions/lib/expression.js | 16 +++++------- packages/expressions/schemas/schema.json | 3 +++ packages/expressions/test/constant.test.js | 5 ++-- packages/expressions/test/expression.test.js | 25 ++++++++++++++++++- packages/expressions/test/schemas.test.js | 4 +-- spec/flipper/gates/expression_spec.rb | 5 ++++ 12 files changed, 58 insertions(+), 21 deletions(-) diff --git a/lib/flipper/expression.rb b/lib/flipper/expression.rb index ed23c4f90..def358003 100644 --- a/lib/flipper/expression.rb +++ b/lib/flipper/expression.rb @@ -22,7 +22,7 @@ def self.build(object) args = args.is_a?(Hash) ? [args] : Array(args) new(name, args.map { |o| build(o) }) - when String, Numeric, FalseClass, TrueClass + when String, Numeric, FalseClass, TrueClass, nil Expression::Constant.new(object) when Symbol Expression::Constant.new(object.to_s) diff --git a/lib/flipper/expressions/percentage.rb b/lib/flipper/expressions/percentage.rb index ec79713a5..bdad81461 100644 --- a/lib/flipper/expressions/percentage.rb +++ b/lib/flipper/expressions/percentage.rb @@ -2,7 +2,7 @@ module Flipper module Expressions class Percentage def self.call(value) - value.to_f.clamp(0, 100) + value.clamp(0, 100) end end end diff --git a/lib/flipper/gates/expression.rb b/lib/flipper/gates/expression.rb index 53d4dc31e..2e3ac91df 100644 --- a/lib/flipper/gates/expression.rb +++ b/lib/flipper/gates/expression.rb @@ -39,7 +39,7 @@ def open?(context) end def protects?(thing) - thing.is_a?(Flipper::Expression) || thing.is_a?(Hash) + thing.is_a?(Flipper::Expression) || thing.is_a?(Flipper::Expression::Constant) || thing.is_a?(Hash) end def wrap(thing) diff --git a/packages/expressions/examples/Equal.json b/packages/expressions/examples/Equal.json index 4531e3c41..aeef35c05 100644 --- a/packages/expressions/examples/Equal.json +++ b/packages/expressions/examples/Equal.json @@ -8,6 +8,14 @@ "expression": { "Equal": ["a", "a"] }, "result": { "enum": [true] } }, + { + "expression": { "Equal": [null, null] }, + "result": { "enum": [true] } + }, + { + "expression": { "Equal": [null, false] }, + "result": { "enum": [false] } + }, { "expression": { "Equal": [1, 2] }, "result": { "enum": [false] } diff --git a/packages/expressions/examples/expressions.json b/packages/expressions/examples/expressions.json index 3ad836e3b..277ac5e8f 100644 --- a/packages/expressions/examples/expressions.json +++ b/packages/expressions/examples/expressions.json @@ -19,10 +19,13 @@ { "expression": 1.1, "result": { "enum": [1.1] } + }, + { + "expression": null, + "result": { "enum": [null] } } ], "invalid": [ - null, {}, [] ] diff --git a/packages/expressions/lib/constant.js b/packages/expressions/lib/constant.js index 8c2fd35de..74e44a435 100644 --- a/packages/expressions/lib/constant.js +++ b/packages/expressions/lib/constant.js @@ -5,7 +5,7 @@ import { Schema } from './schemas' // // Implements the same interface as Expression export class Constant { - constructor (value, { id = uuidv4(), schema = Schema.resolve('#/definitions/constant') } = {}) { + constructor (value, { id = uuidv4(), schema = Schema.resolve('#') } = {}) { this.value = value this.id = id this.schema = schema diff --git a/packages/expressions/lib/expression.js b/packages/expressions/lib/expression.js index 3d9b80e09..de469af37 100644 --- a/packages/expressions/lib/expression.js +++ b/packages/expressions/lib/expression.js @@ -3,13 +3,9 @@ import { Constant } from './constant' import { Schema } from './schemas' function toArray (arg) { - if (Array.isArray(arg)) { - return arg - } else if (arg === null) { - return [] - } else { - return [arg] - } + if (Array.isArray(arg)) return arg + if (arg === null) return [] + return [arg] } // Simple model to transform this: `{ All: [{ Boolean: [true] }]` @@ -20,14 +16,14 @@ export class Expression { return expression } - if (typeof expression === 'object') { + if (['number', 'string', 'boolean'].includes(typeof expression) || expression === null) { + return new Constant(expression, { schema }) + } else if (typeof expression === 'object') { if (Object.keys(expression).length !== 1) { throw new TypeError(`Invalid expression: ${JSON.stringify(expression)}`) } const name = Object.keys(expression)[0] return new Expression({ name, args: expression[name] }) - } else if (['number', 'string', 'boolean'].includes(typeof expression)) { - return new Constant(expression, { schema }) } else { throw new TypeError(`Invalid expression: ${JSON.stringify(expression)}`) } diff --git a/packages/expressions/schemas/schema.json b/packages/expressions/schemas/schema.json index 8812fc08e..ae4d12198 100644 --- a/packages/expressions/schemas/schema.json +++ b/packages/expressions/schemas/schema.json @@ -24,6 +24,9 @@ "title": "Boolean", "type": "boolean", "enum": [true, false] + }, + { + "type": "null" } ] }, diff --git a/packages/expressions/test/constant.test.js b/packages/expressions/test/constant.test.js index 1981f5c1d..67cf5635b 100644 --- a/packages/expressions/test/constant.test.js +++ b/packages/expressions/test/constant.test.js @@ -3,8 +3,8 @@ import { Constant, Schema } from '../lib' describe('Constant', () => { describe('schema', () => { - test('defaults to Constant schema', () => { - expect(new Constant('string').schema.title).toEqual('Constant') + test('defaults to expression schema', () => { + expect(new Constant('string').schema.title).toEqual('Expression') }) test('uses provided schema', () => { @@ -26,7 +26,6 @@ describe('Constant', () => { test('returns false for invalid value', () => { expect(new Constant(['array']).validate().valid).toBe(false) - expect(new Constant({Now: []}).validate().valid).toBe(false) }) test('uses provided schema', () => { diff --git a/packages/expressions/test/expression.test.js b/packages/expressions/test/expression.test.js index bd96cde76..4cf9b2fb3 100644 --- a/packages/expressions/test/expression.test.js +++ b/packages/expressions/test/expression.test.js @@ -11,6 +11,24 @@ describe('Expression', () => { expect(expression.value).toEqual({ All: [true] }) }) + test('builds an expression from a boolean constant', () => { + const expression = Expression.build(true) + expect(expression).toBeInstanceOf(Constant) + expect(expression.value).toEqual(true) + }) + + test('builds an expression from a string constant', () => { + const expression = Expression.build("hello") + expect(expression).toBeInstanceOf(Constant) + expect(expression.value).toEqual("hello") + }) + + test('builds an expression from a null constant', () => { + const expression = Expression.build(null) + expect(expression).toBeInstanceOf(Constant) + expect(expression.value).toEqual(null) + }) + test('throws error on invalid expression', () => { expect(() => Expression.build([])).toThrowError(TypeError) expect(() => Expression.build(new Date())).toThrowError(TypeError) @@ -25,8 +43,13 @@ describe('Expression', () => { expect(expression.args[1].schema).toEqual(schema.items[1]) }) + test('sets schema for constant', () => { + const expression = Expression.build(false) + expect(expression.schema.$id).toEqual(Schema.resolve('#').$id) + }) + test('each subexpression uses its own schema', () => { - const expression = Expression.build({ GreaterThan: [ { Now: [] }, { Property: ['released_at'] } ] }) + const expression = Expression.build({ GreaterThan: [{ Now: [] }, { Property: ['released_at'] }] }) expect(expression.schema).toEqual(Schema.resolve('GreaterThan.schema.json')) expect(expression.args[0].schema).toEqual(Schema.resolve('Now.schema.json')) expect(expression.args[1].schema).toEqual(Schema.resolve('Property.schema.json')) diff --git a/packages/expressions/test/schemas.test.js b/packages/expressions/test/schemas.test.js index cff727813..781e3bbd1 100644 --- a/packages/expressions/test/schemas.test.js +++ b/packages/expressions/test/schemas.test.js @@ -55,12 +55,12 @@ describe('schema.json', () => { describe('resolveAnyOf', () => { test('returns nested anyOf', () => { const ref = Schema.resolve('#') - expect(ref.resolveAnyOf()).toHaveLength(4) + expect(ref.resolveAnyOf()).toHaveLength(5) }) test('returns array of schemas', () => { const ref = Schema.resolve('#/definitions/constant') - expect(ref.resolveAnyOf()).toHaveLength(3) + expect(ref.resolveAnyOf()).toHaveLength(4) expect(ref.resolveAnyOf()).toEqual(ref.anyOf) }) }) diff --git a/spec/flipper/gates/expression_spec.rb b/spec/flipper/gates/expression_spec.rb index 21509d2d7..74361bba7 100644 --- a/spec/flipper/gates/expression_spec.rb +++ b/spec/flipper/gates/expression_spec.rb @@ -83,6 +83,11 @@ def context(expression, properties: {}) expect(subject.protects?(expression)).to be(true) end + it 'returns true for Flipper::Constant' do + expression = Flipper.boolean(true) + expect(subject.protects?(expression)).to be(true) + end + it 'returns true for Hash' do expression = Flipper.number(20).eq(20) expect(subject.protects?(expression.value)).to be(true)