Mobrovac has submitted this change and it was merged.
Change subject: Improve scraper tests, fix additional bugs
......................................................................
Improve scraper tests, fix additional bugs
* Fix bug with scraper test where only open
graph translator was being tested due to the use
of syncronous loops (only async loops work)
* Check creators field for valid fields in scraper
test
* Add function CreatorTypes.prototype.isValidForType
to cachedTypes to check for field validity
* Add unit tests CreatorTypes.prototype.isValidForType
* Fix bugs found by fixed tests
* Whitelist creators in isValidForType and add
corresponding tests.
* Typo: Sponser --> sponsor
* Erroneous publisher fields
Bug: T123360
Change-Id: I667b4e408e2ac61a2b9c23028e63ceef430c3db8
---
M lib/translators/dublinCore.js
M lib/translators/general.js
M lib/zotero/cachedTypes.js
M package.json
M test/features/unit/cachedTypes.js
M test/features/unit/scraper.js
6 files changed, 156 insertions(+), 46 deletions(-)
Approvals:
Mobrovac: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/translators/dublinCore.js b/lib/translators/dublinCore.js
index b3026d2..aac42f8 100644
--- a/lib/translators/dublinCore.js
+++ b/lib/translators/dublinCore.js
@@ -56,14 +56,13 @@
exports.audioRecording = Object.assign({}, extendGeneral('performer'), {
publisher: makeTranslator('label')
});
-exports.bill = extendGeneral('sponser');
+exports.bill = extendGeneral('sponsor');
exports.blogPost = exports.generalWithAuthor;
exports.book = exports.generalWithAuthorAndPublisher;
exports.bookSection = exports.generalWithAuthorAndPublisher;
exports['case'] = {
author: makeCreatorsTranslator('author'),
title: makeTranslator('caseName'),
- date: makeTranslator('date', fixDate),
language: makeTranslator('language', fixLang),
abstract: makeTranslator('abstractNote')
};
@@ -103,8 +102,18 @@
});
exports.newspaperArticle = exports.generalWithAuthor;
exports.note = {}; // Has no fields
-exports.patent = extendGeneral('inventor');
-exports.podcast = extendGeneral('podcaster');
+exports.patent = {
+ author: makeCreatorsTranslator('inventor'),
+ title: makeTranslator('title'),
+ language: makeTranslator('language', fixLang),
+ abstract: makeTranslator('abstractNote')
+};
+exports.podcast = {
+ author: makeCreatorsTranslator('podcaster'),
+ title: makeTranslator('title'),
+ language: makeTranslator('language', fixLang),
+ abstract: makeTranslator('abstractNote')
+};
exports.presentation = extendGeneral('presenter');
exports.radioBroadcast = Object.assign({}, extendGeneral('director'),{
publisher: makeTranslator('network')
@@ -115,7 +124,6 @@
exports.statute = {
author: makeCreatorsTranslator('author'),
title: makeTranslator('nameOfAct'),
- date: makeTranslator('date', fixDate),
language: makeTranslator('language', fixLang),
abstract: makeTranslator('abstractNote')
};
diff --git a/lib/translators/general.js b/lib/translators/general.js
index 12c7053..23865b3 100644
--- a/lib/translators/general.js
+++ b/lib/translators/general.js
@@ -24,22 +24,42 @@
// Create a general type with the author in the creator field once since it is
used many times.
exports.generalWithAuthor = extendGeneral('author');
+exports.generalNoPublisher = {
+ author: makeCreatorsTranslator('author'),
+ canonical: makeTranslator('url'),
+ description: makeTranslator('abstractNote'),
+ title: makeTranslator('title')
+};
/* Complete list of Zotero types with field translators in the Object */
-exports.artwork = extendGeneral('artist');
+exports.artwork = {
+ author: makeCreatorsTranslator('artist'),
+ canonical: makeTranslator('url'),
+ description: makeTranslator('abstractNote'),
+ title: makeTranslator('title')
+};
exports.attachment = {
title: makeTranslator('title')
};
-exports.audioRecording = extendGeneral('performer');
-exports.bill = extendGeneral('sponsor');
-exports.blogPost = exports.generalWithAuthor;
+exports.audioRecording = {
+ author: makeCreatorsTranslator('performer'),
+ canonical: makeTranslator('url'),
+ description: makeTranslator('abstractNote'),
+ title: makeTranslator('title')
+};
+exports.bill = {
+ author: makeCreatorsTranslator('sponsor'),
+ canonical: makeTranslator('url'),
+ description: makeTranslator('abstractNote'),
+ title: makeTranslator('title')
+};
+exports.blogPost = exports.generalNoPublisher;
exports.book = exports.generalWithAuthor;
exports.bookSection = exports.generalWithAuthor;
exports['case'] = {
- author: makeCreatorsTranslator('author'),
+ author: makeCreatorsTranslator('author'),
canonical: makeTranslator('url'),
description: makeTranslator('abstractNote'),
- publisher: makeTranslator('publisher'),
title: makeTranslator('caseName')
};
exports.computerProgram = { // No language field
@@ -56,30 +76,65 @@
title: makeTranslator('subject')
};
exports.encyclopediaArticle = exports.generalWithAuthor;
-exports.film = extendGeneral('director');
-exports.forumPost = exports.generalWithAuthor;
+exports.film = {
+ author: makeCreatorsTranslator('director'),
+ canonical: makeTranslator('url'),
+ description: makeTranslator('abstractNote'),
+ title: makeTranslator('title')
+};
+exports.forumPost = exports.generalNoPublisher;
exports.hearing = extendGeneral('contributor');
-exports.instantMessage = exports.generalWithAuthor;
-exports.interview = extendGeneral('interviewee');
-exports.journalArticle = exports.generalWithAuthor;
-exports.letter = exports.generalWithAuthor;
-exports.magazineArticle = exports.generalWithAuthor;
-exports.manuscript = exports.generalWithAuthor;
+exports.instantMessage = exports.generalNoPublisher;
+exports.interview = {
+ author: makeCreatorsTranslator('interviewee'),
+ canonical: makeTranslator('url'),
+ description: makeTranslator('abstractNote'),
+ title: makeTranslator('title')
+};
+exports.journalArticle = exports.generalNoPublisher;
+exports.letter = exports.generalNoPublisher;
+exports.magazineArticle = exports.generalNoPublisher;
+exports.manuscript = exports.generalNoPublisher;
exports.map = extendGeneral('cartographer');
-exports.newspaperArticle = exports.generalWithAuthor;
+exports.newspaperArticle = {
+ author: makeCreatorsTranslator('author'),
+ canonical: makeTranslator('url'),
+ description: makeTranslator('abstractNote'),
+ title: makeTranslator('title')
+};
exports.note = {}; // Has no fields
-exports.patent = extendGeneral('inventor');
-exports.podcast = extendGeneral('podcaster');
-exports.presentation = extendGeneral('presenter');
-exports.radioBroadcast = extendGeneral('director');
-exports.report = exports.generalWithAuthor;
+exports.patent = {
+ author: makeCreatorsTranslator('inventor'),
+ canonical: makeTranslator('url'),
+ description: makeTranslator('abstractNote'),
+ title: makeTranslator('title')
+};
+exports.podcast = {
+ author: makeCreatorsTranslator('podcaster'),
+ canonical: makeTranslator('url'),
+ description: makeTranslator('abstractNote'),
+ title: makeTranslator('title')
+};
+exports.presentation = {
+ author: makeCreatorsTranslator('presenter'),
+ canonical: makeTranslator('url'),
+ description: makeTranslator('abstractNote'),
+ title: makeTranslator('title')
+};
+exports.radioBroadcast = {
+ author: makeCreatorsTranslator('director'),
+ canonical: makeTranslator('url'),
+ description: makeTranslator('abstractNote'),
+ title: makeTranslator('title')
+};
+exports.report = exports.generalNoPublisher;
exports.statute = {
author: makeCreatorsTranslator('author'),
canonical: makeTranslator('url'),
description: makeTranslator('abstractNote'),
title: makeTranslator('nameOfAct')
};
-exports.thesis = exports.generalWithAuthor;
-exports.tvBroadcast = extendGeneral('director');
-exports.videoRecording = extendGeneral('director');
-exports.webpage = exports.generalWithAuthor;
\ No newline at end of file
+exports.thesis = exports.generalNoPublisher;
+exports.tvBroadcast = exports.radioBroadcast;
+exports.videoRecording = exports.tvBroadcast;
+exports.webpage = exports.generalNoPublisher;
\ No newline at end of file
diff --git a/lib/zotero/cachedTypes.js b/lib/zotero/cachedTypes.js
index fd6a616..83bce48 100644
--- a/lib/zotero/cachedTypes.js
+++ b/lib/zotero/cachedTypes.js
@@ -150,6 +150,16 @@
return itemType[3]/* creatorTypes */[0];
};
+CreatorTypes.prototype.isValidForType = function(fieldIdOrName, typeIdOrName) {
+ var field = this.creatorTypes[fieldIdOrName];
+ var itemType = this.itemTypes[typeIdOrName];
+ // mimics itemFields.js
+ if(!field || !itemType) {return false;}
+
+ /* fields */ /* id */
+ return itemType[3].indexOf(field[0]) !== -1;
+};
+
function ItemFields(fields, itemTypes) {
this.schemaType = "fields";
this.fields = fields;
@@ -158,7 +168,13 @@
}
ItemFields.prototype.isValidForType = function(fieldIdOrName, typeIdOrName) {
- var field = this.fields[fieldIdOrName], itemType =
this.itemTypes[typeIdOrName];
+ var field = this.fields[fieldIdOrName];
+ var itemType = this.itemTypes[typeIdOrName];
+
+ // whitelist creators field when there are valid creator names
+ if (fieldIdOrName === 'creators' && itemType[3].length > 0){
+ return true;
+ }
// mimics itemFields.js
if(!field || !itemType) {return false;}
diff --git a/package.json b/package.json
index 741bcaa..b3d7a65 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
{
"name": "citoid",
- "version": "0.4.1",
+ "version": "0.4.2",
"description": "Converts search terms such as URL or DOI into citations.",
"homepage": "https://www.mediawiki.org/wiki/Citoid";,
"license": "Apache-2.0",
diff --git a/test/features/unit/cachedTypes.js
b/test/features/unit/cachedTypes.js
index becba34..f103c5a 100644
--- a/test/features/unit/cachedTypes.js
+++ b/test/features/unit/cachedTypes.js
@@ -86,6 +86,19 @@
expected = 1;
assert.deepEqual(result, expected);
});
+
+ it('determines if creatorType valid for type- true', function() {
+ result = types.creatorTypesMethods.isValidForType('programmer',
'computerProgram');
+ expected = true;
+ assert.deepEqual(result, expected);
+ });
+
+ it('determines if creatorType valid for type- false', function() {
+ result = types.creatorTypesMethods.isValidForType('elephant',
'computerProgram');
+ expected = false;
+ assert.deepEqual(result, expected);
+ });
+
});
describe('item fields methods', function() {
@@ -101,6 +114,18 @@
assert.deepEqual(result, expected);
});
+ it('determines if creator field is valid for type- true', function() {
+ result = types.itemFieldsMethods.isValidForType('creators',
'webpage');
+ expected = true;
+ assert.deepEqual(result, expected);
+ });
+
+ it('determines if creator field is valid for type- false', function() {
+ result = types.itemFieldsMethods.isValidForType('creators',
'note');
+ expected = true;
+ assert.deepEqual(result, expected);
+ });
+
it('get field id from type and base', function() {
result =
types.itemFieldsMethods.getFieldIDFromTypeAndBase('webpage',
'publicationTitle');
expected = 91;
diff --git a/test/features/unit/scraper.js b/test/features/unit/scraper.js
index 3dffa93..bb740d0 100644
--- a/test/features/unit/scraper.js
+++ b/test/features/unit/scraper.js
@@ -31,28 +31,34 @@
var result;
var expected;
var itemTypeName;
- var metadataType;
// Cycle through every translator
- for (var t in translators){
- metadataType = translators[t];
+ translators.forEach(function(metadataType) {
// Cycle through every sample html file
- for (var file in htmlFiles){
- it('translates '+ metadataType.name +' metadata from ' +
htmlFiles[file].name + ' file', function() {
+ htmlFiles.forEach(function(file) {
+ it('translates '+ metadataType.name +' metadata from ' + file.name
+ ' file', function() {
// Get metadata from html file
- return
meta.parseAll(htmlFiles[file].value).then(function(metadata){
+ return meta.parseAll(file.value).then(function(metadata){
// For every valid Zotero item type, check corresponding
translator on file
Object.keys(itemTypes).forEach(function(key){
- itemTypeName = types.itemTypeMethods.getName(key);
- citation = scraper.translate({},
metadata[metadataType.name], metadataType.value[itemTypeName]);
- // Check that every key in citation is a valid field
for given type
- Object.keys(citation).forEach(function(citationField){
- result =
types.itemFieldsMethods.isValidForType(citationField, itemTypeName);
- assert.deepEqual(result, true, 'Citation field "'
+ citationField + '" is not valid for itemType "' + itemTypeName + '"');
- });
+ // Only test citation if metadata exists for the given
translator type
+ if(metadata[metadataType.name]){
+ citation = scraper.translate({},
metadata[metadataType.name], metadataType.value[itemTypeName]);
+ // Check that every key in citation is a valid
field for given type
+
Object.keys(citation).forEach(function(citationField){
+ result =
types.itemFieldsMethods.isValidForType(citationField, itemTypeName);
+ assert.deepEqual(result, true, 'Citation field
"' + citationField + '" is not valid for itemType "' + itemTypeName + '"');
+ });
+ if (citation.creators){
+ for (var c in citation.creators){
+ result =
types.creatorTypesMethods.isValidForType(citation.creators[c].creatorType,
itemTypeName);
+ assert.deepEqual(result, true, 'Citation
field "' + citation.creators[c].creatorType + '" is not valid for itemType "' +
itemTypeName + '"');
+ }
+ }
+ }
});
});
});
- }
- }
+ });
+ });
});
\ No newline at end of file
--
To view, visit https://gerrit.wikimedia.org/r/263607
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I667b4e408e2ac61a2b9c23028e63ceef430c3db8
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/services/citoid
Gerrit-Branch: master
Gerrit-Owner: Mvolz <[email protected]>
Gerrit-Reviewer: Mobrovac <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
