From a8080bd9b542c21cb0e7be59fb0a229a48d19d0f Mon Sep 17 00:00:00 2001 From: Florian Hotze Date: Sat, 16 Dec 2023 12:49:18 +0100 Subject: [PATCH] [time] Fix `toZDT` fails when injection caching is enabled because instanceof checks don't work (#312) Fixes #288. When `time.toZDT` got a `time.ZonedDateTime` passed in from an external library, it failed to recognize that it in fact got a `time.ZonedDateTime` because instanceof checks were not working due to the webpack settings for the injection caching. Also fixes `utils.isJsInstanceOfJava` not working and renames `utils.isJsInstanceOfJava` to `utils.isJsInstanceOfJavaType`. --------- Signed-off-by: Florian Hotze --- helpers.js | 55 +++++++++++++++++++++++++++++++++++- quantity.js | 40 +++++++++++++------------- test/jest.setup.js | 4 ++- test/time.spec.js | 4 +++ test/utils.spec.js | 35 +++++++++++------------ time.js | 62 ++++++++++++++++++++++------------------- types/quantity.d.ts | 2 +- types/quantity.d.ts.map | 2 +- types/time.d.ts.map | 2 +- types/utils.d.ts | 2 +- types/utils.d.ts.map | 2 +- utils.js | 12 ++++---- 12 files changed, 142 insertions(+), 80 deletions(-) diff --git a/helpers.js b/helpers.js index 44b62dd0f..76a1023b2 100644 --- a/helpers.js +++ b/helpers.js @@ -1,5 +1,9 @@ // Helper functions used internally across the library +const utils = require('./utils'); +const javaZDT = Java.type('java.time.ZonedDateTime'); +const javaDuration = Java.type('java.time.Duration'); + function _getItemName (itemOrName) { // Somehow instanceof check doesn't work here, so workaround the problem if (itemOrName.rawItem !== undefined) return itemOrName.name; @@ -20,7 +24,56 @@ function _isItem (o) { return ((o.constructor && o.constructor.name === 'Item') || typeof o.rawItem === 'object'); } +/** + * Checks whether the given object is an instance of {@link Quantity}. + * + * To be used when instanceof checks don't work because of circular dependencies. + * Checks constructor name or unique properties, because constructor name does not work for the webpacked globals injection. + * + * @param o {*} + * @returns {boolean} + * @private + */ +function _isQuantity (o) { + return ((o.constructor && o.constructor.name === 'Quantity') || typeof o.rawQtyType === 'object'); +} + +/** + * Checks whether the given object is an instance of {@link time.ZonedDateTime}. + * + * To be used when instanceof checks don't work because of circular dependencies. + * Checks constructor name or unique properties, because constructor name does not work for the webpacked globals injection. + * + * @param o {*} + * @returns {boolean} + * @private + */ +function _isZonedDateTime (o) { + return (((o.constructor && o.constructor.name === 'ZonedDateTime')) || + (!utils.isJsInstanceOfJavaType(o, javaZDT) && typeof o.withFixedOffsetZone === 'function') + ); +} + +/** + * Checks whether the given object is an instance of {@link time.Duration}. + * + * To be used when instanceof checks don't work because of circular dependencies. + * Checks constructor name or unique properties, because constructor name does not work for the webpacked globals injection. + * + * @param o {*} + * @returns {boolean} + * @private + */ +function _isDuration (o) { + return (((o.constructor && o.constructor.name === 'Duration')) || + (!utils.isJsInstanceOfJavaType(o, javaDuration) && typeof o.minusDuration === 'function' && typeof o.toNanos === 'function') + ); +} + module.exports = { _getItemName, - _isItem + _isItem, + _isQuantity, + _isZonedDateTime, + _isDuration }; diff --git a/quantity.js b/quantity.js index 0d0294a4e..6168892d3 100644 --- a/quantity.js +++ b/quantity.js @@ -66,7 +66,7 @@ function _toQtyType (value, errorMsg = 'Argument of wrong type provided, require throw new QuantityError(`Failed to create QuantityType from ${value}: ${e}`); } } else if (value instanceof Quantity) { - value = QuantityType.valueOf(value.raw.toString()); // Avoid referencing the same underlying QuantityType, so "clone" it + value = QuantityType.valueOf(value.rawQtyType.toString()); // Avoid referencing the same underlying QuantityType, so "clone" it } else { throw new TypeError(errorMsg); } @@ -105,13 +105,13 @@ class Quantity { * @type {QuantityType} * @private */ - this.raw = value; + this.rawQtyType = value; } else { /** * @type {QuantityType} * @private */ - this.raw = _toQtyType(value); + this.rawQtyType = _toQtyType(value); } } @@ -120,7 +120,7 @@ class Quantity { * @type {string} */ get dimension () { - return this.raw.getDimension().toString(); + return this.rawQtyType.getDimension().toString(); } /** @@ -128,7 +128,7 @@ class Quantity { * @type {string|null} */ get unit () { - const unit = this.raw.getUnit().getName(); + const unit = this.rawQtyType.getUnit().getName(); return (unit === null) ? null : unit.toString(); } @@ -137,7 +137,7 @@ class Quantity { * @type {string|null} */ get symbol () { - const str = this.raw.toString(); + const str = this.rawQtyType.toString(); const i = str.indexOf(' '); return (i !== -1) ? str.substring(i + 1) : null; } @@ -147,7 +147,7 @@ class Quantity { * @type {number} */ get float () { - return parseFloat(this.raw.doubleValue()); + return parseFloat(this.rawQtyType.doubleValue()); } /** @@ -155,7 +155,7 @@ class Quantity { * @type {number} */ get int () { - return parseInt(this.raw.longValue()); + return parseInt(this.rawQtyType.longValue()); } /** @@ -166,7 +166,7 @@ class Quantity { */ add (value) { value = _toQtyType(value); - return new Quantity(this.raw.add(value)); + return new Quantity(this.rawQtyType.add(value)); } /** @@ -181,7 +181,7 @@ class Quantity { */ divide (value) { value = _toBigDecimalOrQtyType(value); - return new Quantity(this.raw.divide(value)); + return new Quantity(this.rawQtyType.divide(value)); } /** @@ -196,7 +196,7 @@ class Quantity { */ multiply (value) { value = _toBigDecimalOrQtyType(value); - return new Quantity(this.raw.multiply(value)); + return new Quantity(this.rawQtyType.multiply(value)); } /** @@ -207,7 +207,7 @@ class Quantity { */ subtract (value) { value = _toQtyType(value); - return new Quantity(this.raw.subtract(value)); + return new Quantity(this.rawQtyType.subtract(value)); } /** @@ -220,12 +220,12 @@ class Quantity { toUnit (unit) { let qtyType; try { - qtyType = (this.raw.toUnit(unit)); + qtyType = (this.rawQtyType.toUnit(unit)); } catch (e) { throw new QuantityError(`Failed to parse unit ${unit}: ${e}`); } if (qtyType === null) { - console.warn(`Failed to convert ${this.raw.toString()} to unit ${unit}.`); + console.warn(`Failed to convert ${this.rawQtyType.toString()} to unit ${unit}.`); return null; } return new Quantity(qtyType); @@ -239,7 +239,7 @@ class Quantity { */ equal (value) { value = _toQtyType(value); - return this.raw.compareTo(value) === 0; + return this.rawQtyType.compareTo(value) === 0; } /** @@ -250,7 +250,7 @@ class Quantity { */ greaterThan (value) { value = _toQtyType(value); - return this.raw.compareTo(value) > 0; + return this.rawQtyType.compareTo(value) > 0; } /** @@ -261,7 +261,7 @@ class Quantity { */ greaterThanOrEqual (value) { value = _toQtyType(value); - return this.raw.compareTo(value) >= 0; + return this.rawQtyType.compareTo(value) >= 0; } /** @@ -272,7 +272,7 @@ class Quantity { */ lessThan (value) { value = _toQtyType(value); - return this.raw.compareTo(value) < 0; + return this.rawQtyType.compareTo(value) < 0; } /** @@ -283,11 +283,11 @@ class Quantity { */ lessThanOrEqual (value) { value = _toQtyType(value); - return this.raw.compareTo(value) <= 0; + return this.rawQtyType.compareTo(value) <= 0; } toString () { - return this.raw.toString(); + return this.rawQtyType.toString(); } } diff --git a/test/jest.setup.js b/test/jest.setup.js index 7d147d06a..108d42d09 100644 --- a/test/jest.setup.js +++ b/test/jest.setup.js @@ -1,9 +1,10 @@ const { ModuleBuilder, Configuration, QuantityType, JavaScriptExecution, JavaTransformation } = require('./openhab.mock'); -const { Class, BigDecimal, ArrayList, HashSet, Hashtable, UUID, FrameworkUtil, LoggerFactory } = require('./java.mock'); +const { Class, BigDecimal, ArrayList, HashSet, Hashtable, UUID, FrameworkUtil, LoggerFactory, ZonedDateTime } = require('./java.mock'); const TYPES = { 'java.lang.Class': Class, 'java.math.BigDecimal': BigDecimal, + 'java.time.ZonedDateTime': ZonedDateTime, 'java.util.ArrayList': ArrayList, 'java.util.HashSet': HashSet, 'java.util.Hashtable': Hashtable, @@ -20,6 +21,7 @@ const TYPES = { /* eslint-disable-next-line no-global-assign */ Java = { type: (type) => TYPES[type], + typeName: jest.fn(), from: jest.fn(), isType: jest.fn(), isJavaObject: jest.fn() diff --git a/test/time.spec.js b/test/time.spec.js index a88be271a..871250a7e 100644 --- a/test/time.spec.js +++ b/test/time.spec.js @@ -5,6 +5,10 @@ jest.mock('../items', () => ({ Item: new Object() // eslint-disable-line no-new-object })); +jest.mock('../utils', () => ({ + isJsInstanceOfJavaType: () => false +})); + describe('time.js', () => { describe('toZDT', () => { it('passes through if when is a time.ZonedDateTime', () => { diff --git a/test/utils.spec.js b/test/utils.spec.js index b7a5de10a..029fa5d2d 100644 --- a/test/utils.spec.js +++ b/test/utils.spec.js @@ -7,7 +7,7 @@ const { javaListToJsArray, javaSetToJsArray, javaSetToJsSet, - isJsInstanceOfJava, + isJsInstanceOfJavaType, javaInstantToJsInstant, javaZDTToJsZDT } = require('../utils'); @@ -90,39 +90,38 @@ describe('utils.js', () => { }); }); - describe('isJsInstanceOfJava', () => { + describe('isJsInstanceOfJavaType', () => { it('throws error when type is not a Java type.', () => { const notAJavaType = {}; jest.spyOn(Java, 'isType').mockImplementation(() => false); - expect(() => isJsInstanceOfJava('', notAJavaType)).toThrow( - 'type is not a java class' + expect(() => isJsInstanceOfJavaType('', notAJavaType)).toThrow( + 'type is not a Java type' ); expect(Java.isType).toHaveBeenCalledWith(notAJavaType); }); - it('returns false if instance oder type is null or undefined.', () => { + it('returns false if instance or type is null or undefined.', () => { jest.spyOn(Java, 'isType').mockImplementation(() => true); - expect(isJsInstanceOfJava(null, {})).toBe(false); - expect(isJsInstanceOfJava(undefined, {})).toBe(false); - expect(isJsInstanceOfJava({ getClass: () => { return null; } }, {})).toBe(false); - expect(isJsInstanceOfJava({ getClass: () => { return undefined; } }, {})).toBe(false); + expect(isJsInstanceOfJavaType(null, {})).toBe(false); + expect(isJsInstanceOfJavaType(undefined, {})).toBe(false); + expect(isJsInstanceOfJavaType({ getClass: () => { return null; } }, {})).toBe(false); + expect(isJsInstanceOfJavaType({ getClass: () => { return undefined; } }, {})).toBe(false); }); - it("delegates to isAssignableFrom of given type's class", () => { - const isAssignableFromMock = jest.fn(); - const javaType = { + it('delegates to Java.typeName(type) and instance.getClass().getName()', () => { + const getNameMock = jest.fn(() => 'java.lang.Object'); + const instance = { getClass: () => { return { - isAssignableFrom: isAssignableFromMock + getName: getNameMock }; } }; - const instance = { getClass: () => 'class' }; + const type = {}; jest.spyOn(Java, 'isType').mockImplementation(() => true); - isJsInstanceOfJava(instance, javaType); - expect(isAssignableFromMock).toHaveBeenCalledWith( - 'class' - ); + jest.spyOn(Java, 'typeName').mockImplementation(() => 'java.lang.Object'); + isJsInstanceOfJavaType(instance, type); + expect(getNameMock).toHaveBeenCalled(); }); }); diff --git a/time.js b/time.js index 4214bb144..9269797a2 100644 --- a/time.js +++ b/time.js @@ -7,17 +7,18 @@ require('@js-joda/timezone'); const time = require('@js-joda/core'); + +const log = require('./log')('time'); const items = require('./items'); const utils = require('./utils'); -const { _isItem } = require('./helpers'); +const { _isItem, _isZonedDateTime, _isDuration, _isQuantity } = require('./helpers'); const javaZDT = Java.type('java.time.ZonedDateTime'); const javaDuration = Java.type('java.time.Duration'); const javaString = Java.type('java.lang.String'); const javaNumber = Java.type('java.lang.Number'); -const { DateTimeType, DecimalType, StringType, QuantityType } = require('@runtime'); -const { Quantity } = require('./quantity'); const ohItem = Java.type('org.openhab.core.items.Item'); +const { DateTimeType, DecimalType, StringType, QuantityType } = require('@runtime'); /** * Adds millis to the passed in ZDT as milliseconds. The millis is rounded first. @@ -200,69 +201,72 @@ function _convertItem (item) { function toZDT (when) { // If when is not supplied or null, return now if (when === undefined || when === null) { + log.debug('toZDT: Returning ZonedDateTime.now()'); return time.ZonedDateTime.now(); } // Pass through if already a time.ZonedDateTime - if (when instanceof time.ZonedDateTime) { + if (_isZonedDateTime(when)) { + log.debug('toZDT: Passing trough ' + when); return when; } + // Convert Java ZonedDateTime + if (when instanceof javaZDT) { + log.debug('toZTD: Converting Java ZonedDateTime ' + when.toString()); + return utils.javaZDTToJsZDTWithDefaultZoneSystem(when); + } // String or StringType if (typeof when === 'string' || when instanceof javaString || when instanceof StringType) { + log.debug('toZDT: Parsing string ' + when); return _parseString(when.toString()); } // JavaScript Native Date, use the SYSTEM timezone if (when instanceof Date) { + log.debug('toZDT: Converting JS native Date ' + when); const native = time.nativeJs(when); const instant = time.Instant.from(native); return time.ZonedDateTime.ofInstant(instant, time.ZoneId.SYSTEM); } // Duration, add to now - if (when instanceof time.Duration || when instanceof javaDuration) { + if (_isDuration(when) || when instanceof javaDuration) { + log.debug('toZDT: Adding duration ' + when + ' to now'); return time.ZonedDateTime.now().plus(time.Duration.parse(when.toString())); } - // JavaScript number of bigint, add as millisecs to now - if (typeof when === 'number' || typeof when === 'bigint' || !isNaN(when)) { + // Add JavaScript's number or JavaScript BigInt or Java Number or Java DecimalType as milliseconds to now + if (typeof when === 'number' || typeof when === 'bigint') { + log.debug('toZDT: Adding milliseconds ' + when + ' to now'); return _addMillisToNow(when); - } - - // Java ZDT - if (when instanceof javaZDT) { - return utils.javaZDTToJsZDTWithDefaultZoneSystem(when); + } else if (when instanceof javaNumber || when instanceof DecimalType) { + log.debug('toZDT: Adding Java number or DecimalType ' + when.floatValue() + ' to now'); + return _addMillisToNow(when.floatValue()); } // DateTimeType, extract the javaZDT and convert to time.ZDT if (when instanceof DateTimeType) { + log.debug('toZTD: Converting DateTimeType ' + when); return utils.javaZDTToJsZDTWithDefaultZoneSystem(when.getZonedDateTime()); } - // Quantity - if (when instanceof Quantity) { + // Add Quantity or QuantityType