From 509fbaee1c7cee9dc04dcd576085d7458be38981 Mon Sep 17 00:00:00 2001 From: tsauerwein Date: Thu, 6 Nov 2014 15:08:34 +0100 Subject: [PATCH] Replace hashCode with checksum Hash codes are not collision free, so what we actually need is a checksum. --- src/ol/structs/checksum.js | 22 +++++++++ src/ol/structs/hashable.js | 16 ------- src/ol/style/circlestyle.js | 36 ++++++++++----- src/ol/style/fillstyle.js | 24 ++++++---- src/ol/style/strokestyle.js | 54 ++++++++++++++-------- test/spec/ol/style/circlestyle.test.js | 64 +++++++++++++++++++++++--- 6 files changed, 154 insertions(+), 62 deletions(-) create mode 100644 src/ol/structs/checksum.js delete mode 100644 src/ol/structs/hashable.js diff --git a/src/ol/structs/checksum.js b/src/ol/structs/checksum.js new file mode 100644 index 0000000000..423da745cf --- /dev/null +++ b/src/ol/structs/checksum.js @@ -0,0 +1,22 @@ +goog.provide('ol.structs.IHasChecksum'); + + +/** + * @typedef {string} + */ +ol.structs.Checksum; + + + +/** + * @interface + */ +ol.structs.IHasChecksum = function() { +}; + + +/** + * @return {string} The checksum. + */ +ol.structs.IHasChecksum.prototype.getChecksum = function() { +}; diff --git a/src/ol/structs/hashable.js b/src/ol/structs/hashable.js deleted file mode 100644 index 371beafa6c..0000000000 --- a/src/ol/structs/hashable.js +++ /dev/null @@ -1,16 +0,0 @@ -goog.provide('ol.structs.IHashable'); - - - -/** - * @interface - */ -ol.structs.IHashable = function() { -}; - - -/** - * @return {number} The hash code. - */ -ol.structs.IHashable.prototype.hashCode = function() { -}; diff --git a/src/ol/style/circlestyle.js b/src/ol/style/circlestyle.js index 235d066290..6a52f22ba6 100644 --- a/src/ol/style/circlestyle.js +++ b/src/ol/style/circlestyle.js @@ -2,10 +2,9 @@ goog.provide('ol.style.Circle'); goog.require('goog.dom'); goog.require('goog.dom.TagName'); -goog.require('goog.string'); goog.require('ol.color'); goog.require('ol.render.canvas'); -goog.require('ol.structs.IHashable'); +goog.require('ol.structs.IHasChecksum'); goog.require('ol.style.Fill'); goog.require('ol.style.Image'); goog.require('ol.style.ImageState'); @@ -20,7 +19,7 @@ goog.require('ol.style.Stroke'); * @constructor * @param {olx.style.CircleOptions=} opt_options Options. * @extends {ol.style.Image} - * @implements {ol.structs.IHashable} + * @implements {ol.structs.IHasChecksum} * @api */ ol.style.Circle = function(opt_options) { @@ -78,6 +77,12 @@ ol.style.Circle = function(opt_options) { */ this.size_ = [size, size]; + /** + * @private + * @type {Array.|null} + */ + this.checksums_ = null; + /** * @type {boolean} */ @@ -268,15 +273,22 @@ ol.style.Circle.prototype.render_ = function() { /** * @inheritDoc */ -ol.style.Circle.prototype.hashCode = function() { - var hash = 17; +ol.style.Circle.prototype.getChecksum = function() { + var strokeChecksum = !goog.isNull(this.stroke_) ? + this.stroke_.getChecksum() : '-'; + var fillChecksum = !goog.isNull(this.fill_) ? + this.fill_.getChecksum() : '-'; - hash = hash * 23 + (!goog.isNull(this.stroke_) ? - this.stroke_.hashCode() : 0); - hash = hash * 23 + (!goog.isNull(this.fill_) ? - this.fill_.hashCode() : 0); - hash = hash * 23 + (goog.isDef(this.radius_) ? - goog.string.hashCode(this.radius_.toString()) : 0); + var recalculate = goog.isNull(this.checksums_) || + (strokeChecksum != this.checksums_[1] || + fillChecksum != this.checksums_[2] || + this.radius_ != this.checksums_[3]); - return hash; + if (recalculate) { + var checksum = 'c' + strokeChecksum + fillChecksum + + (goog.isDef(this.radius_) ? this.radius_.toString() : '-'); + this.checksums_ = [checksum, strokeChecksum, fillChecksum, this.radius_]; + } + + return this.checksums_[0]; }; diff --git a/src/ol/style/fillstyle.js b/src/ol/style/fillstyle.js index 65091029a7..a54d8d2498 100644 --- a/src/ol/style/fillstyle.js +++ b/src/ol/style/fillstyle.js @@ -1,8 +1,7 @@ goog.provide('ol.style.Fill'); -goog.require('goog.string'); goog.require('ol.color'); -goog.require('ol.structs.IHashable'); +goog.require('ol.structs.IHasChecksum'); @@ -12,7 +11,7 @@ goog.require('ol.structs.IHashable'); * * @constructor * @param {olx.style.FillOptions=} opt_options Options. - * @implements {ol.structs.IHashable} + * @implements {ol.structs.IHasChecksum} * @api */ ol.style.Fill = function(opt_options) { @@ -24,6 +23,12 @@ ol.style.Fill = function(opt_options) { * @type {ol.Color|string} */ this.color_ = goog.isDef(options.color) ? options.color : null; + + /** + * @private + * @type {?ol.structs.Checksum} + */ + this.checksum_ = null; }; @@ -44,17 +49,18 @@ ol.style.Fill.prototype.getColor = function() { */ ol.style.Fill.prototype.setColor = function(color) { this.color_ = color; + this.checksum_ = null; }; /** * @inheritDoc */ -ol.style.Fill.prototype.hashCode = function() { - var hash = 17; +ol.style.Fill.prototype.getChecksum = function() { + if (goog.isNull(this.checksum_)) { + this.checksum_ = 'f' + (!goog.isNull(this.color_) ? + ol.color.asString(this.color_) : '-'); + } - hash = hash * 23 + (!goog.isNull(this.color_) ? - goog.string.hashCode(ol.color.asString(this.color_)) : 0); - - return hash; + return this.checksum_; }; diff --git a/src/ol/style/strokestyle.js b/src/ol/style/strokestyle.js index 934490f3b1..419e771cd3 100644 --- a/src/ol/style/strokestyle.js +++ b/src/ol/style/strokestyle.js @@ -1,8 +1,9 @@ goog.provide('ol.style.Stroke'); -goog.require('goog.string'); +goog.require('goog.crypt'); +goog.require('goog.crypt.Md5'); goog.require('ol.color'); -goog.require('ol.structs.IHashable'); +goog.require('ol.structs.IHasChecksum'); @@ -15,7 +16,7 @@ goog.require('ol.structs.IHashable'); * * @constructor * @param {olx.style.StrokeOptions=} opt_options Options. - * @implements {ol.structs.IHashable} + * @implements {ol.structs.IHasChecksum} * @api */ ol.style.Stroke = function(opt_options) { @@ -57,6 +58,12 @@ ol.style.Stroke = function(opt_options) { * @type {number|undefined} */ this.width_ = options.width; + + /** + * @private + * @type {?ol.structs.Checksum} + */ + this.checksum_ = null; }; @@ -122,6 +129,7 @@ ol.style.Stroke.prototype.getWidth = function() { */ ol.style.Stroke.prototype.setColor = function(color) { this.color_ = color; + this.checksum_ = null; }; @@ -133,6 +141,7 @@ ol.style.Stroke.prototype.setColor = function(color) { */ ol.style.Stroke.prototype.setLineCap = function(lineCap) { this.lineCap_ = lineCap; + this.checksum_ = null; }; @@ -144,6 +153,7 @@ ol.style.Stroke.prototype.setLineCap = function(lineCap) { */ ol.style.Stroke.prototype.setLineDash = function(lineDash) { this.lineDash_ = lineDash; + this.checksum_ = null; }; @@ -155,6 +165,7 @@ ol.style.Stroke.prototype.setLineDash = function(lineDash) { */ ol.style.Stroke.prototype.setLineJoin = function(lineJoin) { this.lineJoin_ = lineJoin; + this.checksum_ = null; }; @@ -166,6 +177,7 @@ ol.style.Stroke.prototype.setLineJoin = function(lineJoin) { */ ol.style.Stroke.prototype.setMiterLimit = function(miterLimit) { this.miterLimit_ = miterLimit; + this.checksum_ = null; }; @@ -177,27 +189,33 @@ ol.style.Stroke.prototype.setMiterLimit = function(miterLimit) { */ ol.style.Stroke.prototype.setWidth = function(width) { this.width_ = width; + this.checksum_ = null; }; /** * @inheritDoc */ -ol.style.Stroke.prototype.hashCode = function() { - var hash = 17; +ol.style.Stroke.prototype.getChecksum = function() { + if (goog.isNull(this.checksum_)) { + var raw = 's' + + (!goog.isNull(this.color_) ? + ol.color.asString(this.color_) : '-') + ',' + + (goog.isDef(this.lineCap_) ? + this.lineCap_.toString() : '-') + ',' + + (!goog.isNull(this.lineDash_) ? + this.lineDash_.toString() : '-') + ',' + + (goog.isDef(this.lineJoin_) ? + this.lineJoin_ : '-') + ',' + + (goog.isDef(this.miterLimit_) ? + this.miterLimit_.toString() : '-') + ',' + + (goog.isDef(this.width_) ? + this.width_.toString() : '-'); - hash = hash * 23 + (!goog.isNull(this.color_) ? - goog.string.hashCode(ol.color.asString(this.color_)) : 0); - hash = hash * 23 + (goog.isDef(this.lineCap_) ? - goog.string.hashCode(this.lineCap_.toString()) : 0); - hash = hash * 23 + (!goog.isNull(this.lineDash_) ? - goog.string.hashCode(this.lineDash_.toString()) : 0); - hash = hash * 23 + (goog.isDef(this.lineJoin_) ? - goog.string.hashCode(this.lineJoin_) : 0); - hash = hash * 23 + (goog.isDef(this.miterLimit_) ? - goog.string.hashCode(this.miterLimit_.toString()) : 0); - hash = hash * 23 + (goog.isDef(this.width_) ? - goog.string.hashCode(this.width_.toString()) : 0); + var md5 = new goog.crypt.Md5(); + md5.update(raw); + this.checksum_ = goog.crypt.byteArrayToString(md5.digest()); + } - return hash; + return this.checksum_; }; diff --git a/test/spec/ol/style/circlestyle.test.js b/test/spec/ol/style/circlestyle.test.js index 566164e199..00eb1765a6 100644 --- a/test/spec/ol/style/circlestyle.test.js +++ b/test/spec/ol/style/circlestyle.test.js @@ -3,12 +3,12 @@ goog.provide('ol.test.style.Circle'); describe('ol.style.Circle', function() { - describe('#hashCode', function() { + describe('#getChecksum', function() { it('calculates the same hash code for default options', function() { var style1 = new ol.style.Circle(); var style2 = new ol.style.Circle(); - expect(style1.hashCode()).to.eql(style2.hashCode()); + expect(style1.getChecksum()).to.eql(style2.getChecksum()); }); it('calculates not the same hash code (radius)', function() { @@ -16,7 +16,7 @@ describe('ol.style.Circle', function() { var style2 = new ol.style.Circle({ radius: 5 }); - expect(style1.hashCode()).to.not.eql(style2.hashCode()); + expect(style1.getChecksum()).to.not.eql(style2.getChecksum()); }); it('calculates the same hash code (radius)', function() { @@ -26,7 +26,7 @@ describe('ol.style.Circle', function() { var style2 = new ol.style.Circle({ radius: 5 }); - expect(style1.hashCode()).to.eql(style2.hashCode()); + expect(style1.getChecksum()).to.eql(style2.getChecksum()); }); it('calculates not the same hash code (color)', function() { @@ -42,7 +42,7 @@ describe('ol.style.Circle', function() { color: '#319FD3' }) }); - expect(style1.hashCode()).to.not.eql(style2.hashCode()); + expect(style1.getChecksum()).to.not.eql(style2.getChecksum()); }); it('calculates the same hash code (everything set)', function() { @@ -74,7 +74,7 @@ describe('ol.style.Circle', function() { width: 2 }) }); - expect(style1.hashCode()).to.eql(style2.hashCode()); + expect(style1.getChecksum()).to.eql(style2.getChecksum()); }); it('calculates not the same hash code (stroke width differs)', function() { @@ -106,7 +106,57 @@ describe('ol.style.Circle', function() { width: 2 }) }); - expect(style1.hashCode()).to.not.eql(style2.hashCode()); + expect(style1.getChecksum()).to.not.eql(style2.getChecksum()); + }); + + it('invalidates a cached checksum if values change (fill)', function() { + var style1 = new ol.style.Circle({ + radius: 5, + fill: new ol.style.Fill({ + color: '#319FD3' + }), + stroke: new ol.style.Stroke({ + color: '#319FD3' + }) + }); + var style2 = new ol.style.Circle({ + radius: 5, + fill: new ol.style.Fill({ + color: '#319FD3' + }), + stroke: new ol.style.Stroke({ + color: '#319FD3' + }) + }); + expect(style1.getChecksum()).to.eql(style2.getChecksum()); + + style1.getFill().setColor('red'); + expect(style1.getChecksum()).to.not.eql(style2.getChecksum()); + }); + + it('invalidates a cached checksum if values change (stroke)', function() { + var style1 = new ol.style.Circle({ + radius: 5, + fill: new ol.style.Fill({ + color: '#319FD3' + }), + stroke: new ol.style.Stroke({ + color: '#319FD3' + }) + }); + var style2 = new ol.style.Circle({ + radius: 5, + fill: new ol.style.Fill({ + color: '#319FD3' + }), + stroke: new ol.style.Stroke({ + color: '#319FD3' + }) + }); + expect(style1.getChecksum()).to.eql(style2.getChecksum()); + + style1.getStroke().setWidth(4); + expect(style1.getChecksum()).to.not.eql(style2.getChecksum()); }); });