From 6e2ec62b00793e6a24629d1bf90186d614f50681 Mon Sep 17 00:00:00 2001 From: David Luis Wiegandt Date: Tue, 24 Nov 2015 12:53:18 +0100 Subject: [PATCH] feat(Viewer): indicate empty model through an error Fail import with an error message if the XML does not contain a valid diagram. Closes #417 --- lib/Viewer.js | 6 ++-- lib/import/BpmnTreeWalker.js | 12 +++++-- test/spec/ModelerSpec.js | 17 ++++++++-- test/spec/ViewerSpec.js | 57 ++++++++++++++++++++------------- test/spec/util/ModelUtilSpec.js | 2 +- 5 files changed, 62 insertions(+), 32 deletions(-) diff --git a/lib/Viewer.js b/lib/Viewer.js index 998cf6e4..436fef36 100644 --- a/lib/Viewer.js +++ b/lib/Viewer.js @@ -172,11 +172,9 @@ Viewer.prototype.importXML = function(xml, done) { var parseWarnings = context.warnings; self.importDefinitions(definitions, function(err, importWarnings) { - if (err) { - return done(err); - } + var allWarnings = parseWarnings.concat(importWarnings || []); - done(null, parseWarnings.concat(importWarnings || [])); + done(err, allWarnings); }); }); }; diff --git a/lib/import/BpmnTreeWalker.js b/lib/import/BpmnTreeWalker.js index 8ce175ec..2549c374 100644 --- a/lib/import/BpmnTreeWalker.js +++ b/lib/import/BpmnTreeWalker.js @@ -130,6 +130,14 @@ function BpmnTreeWalker(handler) { ////// Semantic handling ////////////////////// + /** + * Handle definitions and return the rendered diagram (if any) + * + * @param {ModdleElement} definitions to walk and import + * @param {ModdleElement} [diagram] specific diagram to import and display + * + * @throws {Error} if no diagram to display could be found + */ function handleDefinitions(definitions, diagram) { // make sure we walk the correct bpmnElement @@ -145,7 +153,7 @@ function BpmnTreeWalker(handler) { // no diagram -> nothing to import if (!diagram) { - return; + throw new Error('no diagram to display'); } // load DI from selected diagram only @@ -167,7 +175,7 @@ function BpmnTreeWalker(handler) { rootElement = findDisplayCandidate(definitions); if (!rootElement) { - return logError('no process or collaboration present to display'); + throw new Error('no process or collaboration to display'); } else { logError('correcting missing bpmnElement on ' + elementToString(plane) + ' to ' + elementToString(rootElement)); diff --git a/test/spec/ModelerSpec.js b/test/spec/ModelerSpec.js index 51dd9155..27586d62 100644 --- a/test/spec/ModelerSpec.js +++ b/test/spec/ModelerSpec.js @@ -40,9 +40,22 @@ describe('Modeler', function() { }); - it('should import empty definitions', function(done) { + it('should not import empty definitions', function(done) { var xml = require('../fixtures/bpmn/empty-definitions.bpmn'); - createModeler(xml, done); + + // given + createModeler(xml, function(err, warnings, modeler) { + + // when + modeler.importXML(xml, function(err, warnings) { + + // then + expect(err.message).to.equal('no diagram to display'); + + done(); + }); + + }); }); diff --git a/test/spec/ViewerSpec.js b/test/spec/ViewerSpec.js index 166de020..4944c6f0 100644 --- a/test/spec/ViewerSpec.js +++ b/test/spec/ViewerSpec.js @@ -29,12 +29,6 @@ describe('Viewer', function() { }); - it('should import empty definitions', function(done) { - var xml = require('../fixtures/bpmn/empty-definitions.bpmn'); - createViewer(xml, done); - }); - - it('should re-import simple process', function(done) { var xml = require('../fixtures/bpmn/simple.bpmn'); @@ -51,7 +45,7 @@ describe('Viewer', function() { viewer.importXML(xml, function(err, warnings) { // then - expect(err).to.not.be.ok; + expect(err).to.exist; expect(warnings.length).to.equal(0); done(); @@ -87,7 +81,7 @@ describe('Viewer', function() { // given var viewer = new Viewer({ container: container }); - var xml = require('../fixtures/bpmn/empty-definitions.bpmn'); + var xml = require('../fixtures/bpmn/simple.bpmn'); var events = []; @@ -126,7 +120,7 @@ describe('Viewer', function() { // given var viewer = new Viewer({ container: container }); - var xml = require('../fixtures/bpmn/empty-definitions.bpmn'); + var xml = require('../fixtures/bpmn/simple.bpmn'); // when viewer.on('foo', 1000, function() { @@ -187,7 +181,6 @@ describe('Viewer', function() { describe('error handling', function() { - function expectMessage(e, expectedMessage) { expect(e).to.exist; @@ -215,7 +208,7 @@ describe('Viewer', function() { createViewer(xml, function(err) { - expect(err).to.be.ok; + expect(err).to.exist; expectMessage(err, /Text data outside of root node./); @@ -232,7 +225,7 @@ describe('Viewer', function() { createViewer(xml, function(err, warnings) { // then - expect(err).to.not.be.ok; + expect(err).not.to.exist; expectWarnings(warnings, [ 'unresolved reference ', @@ -255,7 +248,7 @@ describe('Viewer', function() { createViewer(xml, function(err, warnings) { // then - expect(err).to.not.be.ok; + expect(err).not.to.exist; expectWarnings(warnings, [ /unparsable content detected/, @@ -275,14 +268,14 @@ describe('Viewer', function() { createViewer(xml, function(err, warnings) { // then - expect(err).to.not.be.ok; + expect(err).to.exist; + expect(err.message).to.eql('no process or collaboration to display'); expectWarnings(warnings, [ /unparsable content detected/, 'unresolved reference ', 'no bpmnElement referenced in ', - 'no bpmnElement referenced in ', - 'no process or collaboration present to display' + 'no bpmnElement referenced in ' ]); done(); @@ -337,7 +330,7 @@ describe('Viewer', function() { it('should export svg', function(done) { // given - var xml = require('../fixtures/bpmn/empty-definitions.bpmn'); + var xml = require('../fixtures/bpmn/simple.bpmn'); createViewer(xml, function(err, warnings, viewer) { @@ -451,7 +444,7 @@ describe('Viewer', function() { ]; // given - var xml = require('../fixtures/bpmn/empty-definitions.bpmn'); + var xml = require('../fixtures/bpmn/simple.bpmn'); var viewer; @@ -459,7 +452,6 @@ describe('Viewer', function() { viewer.destroy(); }); - it('should override default modules', function(done) { // given @@ -539,19 +531,38 @@ describe('Viewer', function() { var extensionElements = sendTask.extensionElements; // receive task should be moddle extended - expect(sendTask.$instanceOf('camunda:ServiceTaskLike')).to.be.ok; + expect(sendTask.$instanceOf('camunda:ServiceTaskLike')).to.exist; // extension elements should provide typed element - expect(extensionElements).to.be.ok; + expect(extensionElements).to.exist; expect(extensionElements.values.length).to.equal(1); - expect(extensionElements.values[0].$instanceOf('camunda:InputOutput')).to.be.ok; + expect(extensionElements.values[0].$instanceOf('camunda:InputOutput')).to.exist; done(err); }); }); + + it('should throw error due to missing diagram', function(done) { + + var xml = require('../fixtures/bpmn/empty-definitions.bpmn'); + + // given + viewer = new Viewer({ container: container, additionalModules: testModules }); + + // when + viewer.importXML(xml, function(err) { + + // then + expect(err.message).to.eql('no diagram to display'); + + done(); + }); + + }); + }); @@ -568,7 +579,7 @@ describe('Viewer', function() { viewer.destroy(); // then - expect(viewer.container.parentNode).to.not.be.ok; + expect(viewer.container.parentNode).not.to.exist; }); }); diff --git a/test/spec/util/ModelUtilSpec.js b/test/spec/util/ModelUtilSpec.js index 6d965636..5e7e7390 100644 --- a/test/spec/util/ModelUtilSpec.js +++ b/test/spec/util/ModelUtilSpec.js @@ -12,7 +12,7 @@ var ModelUtil = require('../../../lib/util/ModelUtil'); describe('ModelUtil', function() { - var diagramXML = require('../../fixtures/bpmn/empty-definitions.bpmn'); + var diagramXML = require('../../fixtures/bpmn/simple.bpmn'); beforeEach(bootstrapModeler(diagramXML, { modules: [ coreModule, modelingModule ] }));