Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add hash to Snak #77

Merged
merged 1 commit into from
Aug 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ at the heart of the [Wikibase software](http://wikiba.se/).

## Release notes

### 3.1.0 (dev)

* Added a `hash` parameter to the constructors of
`Snak`, `PropertyValueSnak`, `PropertySomeValueSnak` and `PropertyNoValueSnak`,
as well as a `getHash()` function to the `Snak` class.

### 3.0.1 (2016-09-09)

* Fix an issue with MediaWiki loading (init.php)
Expand Down
1 change: 1 addition & 0 deletions src/PropertyNoValueSnak.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ var PARENT = wb.datamodel.Snak;
* @constructor
*
* @param {string} propertyId
* @param {string|null} [hash=null]
*/
var SELF
= wb.datamodel.PropertyNoValueSnak
Expand Down
1 change: 1 addition & 0 deletions src/PropertySomeValueSnak.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ var PARENT = wb.datamodel.Snak;
* @constructor
*
* @param {string} propertyId
* @param {string|null} [hash=null]
*/
var SELF
= wb.datamodel.PropertySomeValueSnak
Expand Down
5 changes: 3 additions & 2 deletions src/PropertyValueSnak.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,18 @@ var PARENT = wb.datamodel.Snak;
*
* @param {string} propertyId
* @param {dataValues.DataValue} value
* @param {string|null} [hash=null]
*
* @throws {Error} value is not a dataValues.DataValue instance.
*/
var SELF = wb.datamodel.PropertyValueSnak = util.inherit(
'WbDataModelPropertyValueSnak',
PARENT,
function( propertyId, value ) {
function( propertyId, value, hash ) {
if( !( value instanceof dv.DataValue ) ) {
throw new Error( 'The value has to be an instance of dataValues.DataValue' );
}
PARENT.call( this, propertyId );
PARENT.call( this, propertyId, hash );
this._value = value;
},
{
Expand Down
21 changes: 20 additions & 1 deletion src/Snak.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,19 @@
* @constructor
*
* @param {string} propertyId
* @param {string|null} [hash=null]
*
* @throws {Error} when trying to instantiate an abstract Snak object.
* @throws {Error} when the property id is omitted.
*/
var SELF = wb.datamodel.Snak = function WbDataModelSnak( propertyId ) {
var SELF = wb.datamodel.Snak = function WbDataModelSnak( propertyId, hash ) {
if( !this.constructor.TYPE ) {
throw new Error( 'Can not create abstract Snak of no specific type' );
} else if( !propertyId ) {
throw new Error( 'Property ID is required for constructing new Snak' );
}
this._propertyId = propertyId;
this._hash = hash || null;
};

/**
Expand All @@ -42,6 +44,12 @@ $.extend( SELF.prototype, {
*/
_propertyId: null,

/**
* @property {string|null}
* @private
*/
_hash: null,

/**
* Returns the Snak type.
*
Expand All @@ -60,6 +68,17 @@ $.extend( SELF.prototype, {
return this._propertyId;
},

/**
* Returns the hash of this Snak.
* Can be null if this Snak was constructed client-side,
* since hashes can only be computed server-side.
*
* @return {string|null}
*/
getHash: function() {
return this._hash;
},

/**
* @param {*} snak
* @return {boolean}
Expand Down
20 changes: 13 additions & 7 deletions tests/Snak.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
QUnit.module( 'wikibase.datamodel.Snak' );

var testSets = [
[wb.datamodel.PropertyNoValueSnak, ['P1']],
[wb.datamodel.PropertyNoValueSnak, ['P2']],
[wb.datamodel.PropertySomeValueSnak, ['P1']],
[wb.datamodel.PropertyValueSnak, ['P1', new dv.StringValue( 'test' )]],
[wb.datamodel.PropertyValueSnak, ['P2', new dv.StringValue( 'test' )]]
[wb.datamodel.PropertyNoValueSnak, ['P1', undefined]],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the two "undefined" can be omitted, but it doesn't hurt to have them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, they’re necessary because the expected hash is the last array element (array[array.length-1]).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks! I see it now.

[wb.datamodel.PropertyNoValueSnak, ['P2', 'hash']],
[wb.datamodel.PropertySomeValueSnak, ['P1', 'hash']],
[wb.datamodel.PropertyValueSnak, ['P1', new dv.StringValue( 'test' ), undefined]],
[wb.datamodel.PropertyValueSnak, ['P2', new dv.StringValue( 'test' ), 'hash']]
];

/**
Expand All @@ -21,11 +21,11 @@ var testSets = [
* @return {wikibase.datamodel.Snak}
*/
function constructSnak( SnakConstructor, params ) {
return new SnakConstructor( params[0], params[1] );
return new SnakConstructor( params[0], params[1], params[2] );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty awkward, because this code always passes three constructor arguments, even if there are only two for two of the subclasses. But the test setup was like this before and the change made in this patch doesn't make it worse.

}

QUnit.test( 'Constructor', function( assert ) {
assert.expect( 15 );
assert.expect( 20 );

for( var i = 0; i < testSets.length; i++ ) {
var SnakConstructor = testSets[i][0],
Expand All @@ -48,6 +48,12 @@ QUnit.test( 'Constructor', function( assert ) {
SnakConstructor.TYPE,
'Test set #' + i + ': Snak type "' + snak.getType() + '" was set correctly.'
);

assert.strictEqual(
snak.getHash(),
snakParams[snakParams.length - 1] || null,
'Test set #' + i + ': Snak hash was set correctly.'
);
}
} );

Expand Down