Skip to content

Commit

Permalink
[time] Fix toZDT fails when injection caching is enabled because in…
Browse files Browse the repository at this point in the history 
…stanceof 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 <florianh_dev@icloud.com>
  • Loading branch information
@florian-h05
florian-h05 authored Dec 16, 2023
1 parent 7602970 commit a8080bd
Show file tree
Hide file tree
Showing 12 changed files with 142 additions and 80 deletions.
55 changes: 54 additions & 1 deletion helpers.js
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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
};
40 changes: 20 additions & 20 deletions quantity.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -120,15 +120,15 @@ class Quantity {
* @type {string}
*/
get dimension () {
return this.raw.getDimension().toString();
return this.rawQtyType.getDimension().toString();
}

/**
* Unit of this Quantity, e.g. `Metre`, or `null` if not available
* @type {string|null}
*/
get unit () {
const unit = this.raw.getUnit().getName();
const unit = this.rawQtyType.getUnit().getName();
return (unit === null) ? null : unit.toString();
}

Expand All @@ -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;
}
Expand All @@ -147,15 +147,15 @@ class Quantity {
* @type {number}
*/
get float () {
return parseFloat(this.raw.doubleValue());
return parseFloat(this.rawQtyType.doubleValue());
}

/**
* Integer (non-decimal number) value of this Quantity
* @type {number}
*/
get int () {
return parseInt(this.raw.longValue());
return parseInt(this.rawQtyType.longValue());
}

/**
Expand All @@ -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));
}

/**
Expand All @@ -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));
}

/**
Expand All @@ -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));
}

/**
Expand All @@ -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));
}

/**
Expand All @@ -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);
Expand All @@ -239,7 +239,7 @@ class Quantity {
*/
equal (value) {
value = _toQtyType(value);
return this.raw.compareTo(value) === 0;
return this.rawQtyType.compareTo(value) === 0;
}

/**
Expand All @@ -250,7 +250,7 @@ class Quantity {
*/
greaterThan (value) {
value = _toQtyType(value);
return this.raw.compareTo(value) > 0;
return this.rawQtyType.compareTo(value) > 0;
}

/**
Expand All @@ -261,7 +261,7 @@ class Quantity {
*/
greaterThanOrEqual (value) {
value = _toQtyType(value);
return this.raw.compareTo(value) >= 0;
return this.rawQtyType.compareTo(value) >= 0;
}

/**
Expand All @@ -272,7 +272,7 @@ class Quantity {
*/
lessThan (value) {
value = _toQtyType(value);
return this.raw.compareTo(value) < 0;
return this.rawQtyType.compareTo(value) < 0;
}

/**
Expand All @@ -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();
}
}

Expand Down
4 changes: 3 additions & 1 deletion test/jest.setup.js
View file
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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()
Expand Down
4 changes: 4 additions & 0 deletions test/time.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
35 changes: 17 additions & 18 deletions test/utils.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const {
javaListToJsArray,
javaSetToJsArray,
javaSetToJsSet,
isJsInstanceOfJava,
isJsInstanceOfJavaType,
javaInstantToJsInstant,
javaZDTToJsZDT
} = require('../utils');
Expand Down Expand Up @@ -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();
});
});

Expand Down
Loading

0 comments on commit a8080bd

Please sign in to comment.