Skip to content

Commit

Permalink
Displays the unassigned relation error in case of a join explanation (#…
Browse files Browse the repository at this point in the history
…303)

## What is the goal of this PR?
The checking of unassigned relations in the when body of a rule that's being explained, now is also performed on the underlying explanation object (where the first retrieved explanation is of type join)

## What are the changes implemented in this PR?
resolves #299
  • Loading branch information
Soroush Saffari authored and sorsaffari committed Jul 2, 2020
1 parent 7518ffc commit 458ca22
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 113 deletions.
34 changes: 0 additions & 34 deletions src/renderer/components/Visualiser/VisualiserGraphBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,39 +2,6 @@ import Style from './Style';
import NodeSettings from './RightBar/SettingsTab/DisplaySettings';
import CDB from '../shared/CanvasDataBuilder';

// Map graql variables and explanations to each concept
function attachExplanation(result) {
return result.map((x) => {
const keys = Array.from(x.map().keys());

if (x.explanation() && x.explanation().queryPattern() === '') { // if explantion is formed from a conjuction go one level deeper and attach explanations for each answer individually
return Array.from(x.explanation().answers()).map((ans) => {
const exp = ans.explanation();
const concepts = [];

ans.map().forEach((concept, key) => {
if (keys.includes(key)) { // only return those concepts which were asked for since the explanation.map() contains all the concepts in the query. e.g. (match $x isa person; $y isa company; get $x; => only $x should be returned)
concept.explanation = exp;
concept.graqlVar = key;
concepts.push(concept);
}
});
return concepts;
}).flatMap(x => x);
}

// else explanation of query respose is same for all concepts in map
const exp = x.explanation();
const key = x.map().keys().next().value;

return Array.from(x.map().values()).flatMap((concept) => {
concept.explanation = exp;
concept.graqlVar = key;
return concept;
});
}).flatMap(x => x);
}

function buildValue(array) {
if (!array) return '';
return array.join(', ');
Expand Down Expand Up @@ -196,5 +163,4 @@ export default {
buildFromConceptList,
prepareNodes,
relationsRolePlayers,
attachExplanation,
};
118 changes: 62 additions & 56 deletions src/renderer/components/Visualiser/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,72 +230,78 @@ export default {
throw e;
}
},
// eslint-disable-next-line consistent-return
async [EXPLAIN_CONCEPT]({ state, getters, commit, rootState }) {
try {
const graknTx = global.graknTx[rootState.activeTab];
const isRelUnassigned = (when) => {
let isRelUnassigned = false;
const relRegex = /(\$[^\s]*|;|{)(\s*?\(.*?\))/g;
let relMatches = relRegex.exec(when);

while (relMatches) {
if (!relMatches[1].includes('$')) {
isRelUnassigned = true;
break;
}
relMatches = relRegex.exec(when);
}

const node = getters.selectedNode;
const queryPattern = node.queryPattern;
const queryPatternVariales = Array.from(new Set(queryPattern.match(/\$[^\s|)|;|,]*/g))).map(x => x.substring(1));
return isRelUnassigned;
};

const explanation = await node.explanation();
const errorUnassigneRels = ruleLabel => (
// eslint-disable-next-line max-len
`The 'when' body of the rule [${ruleLabel}] contains at least one unassigned relation. To see the full explanation for this concept, please redefine the rule with relation variables.`
);

const graknTx = global.graknTx[rootState.activeTab];
const node = getters.selectedNode;
const originalExpl = await node.explanation();
const rule = originalExpl.getRule();
const isExplJoin = !rule;
let finalExplAnswers;

if (!isExplJoin) {
const when = await rule.getWhen();
const label = await rule.label();
if (isRelUnassigned(when)) {
commit('setGlobalErrorMsg', errorUnassigneRels(label));
return false;
}

let isRelUnassigned = false;
const rule = await explanation.getRule();
const when = rule && await rule.getWhen();
const relRegex = /(\$[^\s]*|;|{)(\s*?\(.*?\))/g;
let relMatches = relRegex.exec(when);
while (relMatches) {
if (!relMatches[1].includes('$')) {
isRelUnassigned = true;
break;
finalExplAnswers = originalExpl.getAnswers();
} else {
const ruleExpl = await Promise.all(originalExpl.getAnswers().map(answer => (answer.hasExplanation() ? answer.explanation() : null)).filter(x => x));
const ruleDetails = await Promise.all(ruleExpl.map((explanation) => {
const rule = explanation.getRule();
return Promise.all([rule.label(), rule.getWhen()]);
}));

const violatingRule = ruleDetails.find(([, when]) => isRelUnassigned(when));
if (violatingRule) {
commit('setGlobalErrorMsg', errorUnassigneRels(violatingRule.label));
return false;
}
relMatches = relRegex.exec(when);

finalExplAnswers = ruleExpl.map(expl => expl.getAnswers()).reduce(collect, []);
}

if (isRelUnassigned) {
commit(
'setGlobalErrorMsg',
'The rule `when` definition for this inferred concept contains at least one unassigned relation. At the moment explanation cannot be provided for such a rule.',
);
if (finalExplAnswers.length > 0) {
const data = await CDB.buildInstances(finalExplAnswers);
const rpData = await CDB.buildRPInstances(finalExplAnswers, data, false, graknTx);
data.nodes.push(...rpData.nodes);
data.edges.push(...rpData.edges);

state.visFacade.addToCanvas(data);
commit('updateCanvasData');
const nodesWithAttributes = await computeAttributes(data.nodes, graknTx);

state.visFacade.updateNode(nodesWithAttributes);
const styledEdges = data.edges.map(edge => ({ ...edge, label: edge.hiddenLabel, ...state.visStyle.computeExplanationEdgeStyle() }));
state.visFacade.updateEdge(styledEdges);
commit('loadingQuery', false);
} else {
const explanationAnswers = explanation.getAnswers();
const explanationPromises = [];
const explanationResult = [];

explanationAnswers.forEach((answer) => {
const answerVariabes = Array.from(answer.map().keys());

const isJointExplanation = answerVariabes.every(variable => queryPatternVariales.includes(variable));
if (answer.hasExplanation() && isJointExplanation) {
explanationPromises.push(answer.explanation());
} else if (!isJointExplanation) {
explanationResult.push(answer);
}
});

(await Promise.all(explanationPromises)).map(expl => expl.getAnswers()).reduce(collect, []).forEach((expl) => {
explanationResult.push(expl);
});

if (explanationResult.length > 0) {
const data = await CDB.buildInstances(explanationResult);
const rpData = await CDB.buildRPInstances(explanationResult, data, false, graknTx);
data.nodes.push(...rpData.nodes);
data.edges.push(...rpData.edges);

state.visFacade.addToCanvas(data);
commit('updateCanvasData');
const nodesWithAttributes = await computeAttributes(data.nodes, graknTx);

state.visFacade.updateNode(nodesWithAttributes);
const styledEdges = data.edges.map(edge => ({ ...edge, label: edge.hiddenLabel, ...state.visStyle.computeExplanationEdgeStyle() }));
state.visFacade.updateEdge(styledEdges);
commit('loadingQuery', false);
} else {
commit('setGlobalErrorMsg', 'The transaction has been refreshed since the loading of this node and, as a result, the explaination is incomplete.');
}
commit('setGlobalErrorMsg', 'The transaction has been refreshed since the loading of this node and, as a result, the explaination is incomplete.');
}
} catch (e) {
await reopenTransaction(rootState, commit);
Expand Down
23 changes: 10 additions & 13 deletions src/renderer/components/shared/CanvasDataBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ const getNodeLabelWithAttrs = async (baseLabel, type, instance) => {
* @param {String} graqlVar
* @param {ConceptMap[]} explanation
*/
const buildCommonInstanceNode = (instance, graqlVar, explanation, queryPattern) => {
const buildCommonInstanceNode = (instance, graqlVar, explanation) => {
const node = {};
node.id = instance.id;
node.baseType = instance.baseType;
Expand All @@ -137,7 +137,6 @@ const buildCommonInstanceNode = (instance, graqlVar, explanation, queryPattern)
node.attributes = convertToRemote(instance).attributes;
if (node.isInferred) {
node.explanation = explanation;
node.queryPattern = queryPattern();
}

return node;
Expand All @@ -149,8 +148,8 @@ const buildCommonInstanceNode = (instance, graqlVar, explanation, queryPattern)
* @param {String} graqlVar
* @param {ConceptMap[]} explanation
*/
const getInstanceNode = async (instance, graqlVar, explanation, queryPattern) => {
const node = buildCommonInstanceNode(instance, graqlVar, explanation, queryPattern);
const getInstanceNode = async (instance, graqlVar, explanation) => {
const node = buildCommonInstanceNode(instance, graqlVar, explanation);
switch (instance.baseType) {
case ENTITY_INSTANCE: {
node.label = await getNodeLabelWithAttrs(`${node.type}: ${node.id}`, node.type, instance);
Expand Down Expand Up @@ -237,12 +236,11 @@ const getInstanceEdges = async (instance, existingNodeIds) => {
*/
const buildInstances = async (answers) => {
let data = answers.map((answerGroup) => {
const { explanation, queryPattern } = answerGroup;
const { explanation } = answerGroup;
return Array.from(answerGroup.map().entries()).map(([graqlVar, concept]) => ({
graqlVar,
concept,
explanation,
queryPattern,
}));
}).reduce(collect, []);

Expand All @@ -254,7 +252,7 @@ const buildInstances = async (answers) => {

data = deduplicateConcepts(data);

const nodes = (await Promise.all(data.filter(item => item.shouldVisualise).map(item => getInstanceNode(item.concept, item.graqlVar, item.explanation, item.queryPattern))))
const nodes = (await Promise.all(data.filter(item => item.shouldVisualise).map(item => getInstanceNode(item.concept, item.graqlVar, item.explanation))))
.reduce(collect, []);
const nodeIds = nodes.map(node => node.id);
const edges = (await Promise.all(data.filter(item => item.shouldVisualise).map(item => getInstanceEdges(item.concept, nodeIds)))).reduce(collect, []);
Expand Down Expand Up @@ -421,12 +419,12 @@ const buildTypes = async (answers) => {
* @param {*} graqlVar
* @param {*} explanation
*/
const getNeighbourNode = (concept, graqlVar, explanation, queryPattern) => {
const getNeighbourNode = (concept, graqlVar, explanation) => {
let node;
if (concept.isType()) {
node = getTypeNode(concept, graqlVar);
} else if (concept.isThing()) {
node = getInstanceNode(concept, graqlVar, explanation, queryPattern);
node = getInstanceNode(concept, graqlVar, explanation);
}
return node;
};
Expand Down Expand Up @@ -481,12 +479,11 @@ const getNeighbourEdges = async (neighbourConcept, targetConcept, existingNodeId
*/
const buildNeighbours = async (targetConcept, answers) => {
let data = answers.map((answerGroup) => {
const { explanation, queryPattern } = answerGroup;
const { explanation } = answerGroup;
return Array.from(answerGroup.map().entries()).map(([graqlVar, concept]) => ({
graqlVar,
concept,
explanation,
queryPattern,
}));
}).reduce(collect, []);

Expand All @@ -499,7 +496,7 @@ const buildNeighbours = async (targetConcept, answers) => {

data = deduplicateConcepts(data);

const nodes = (await Promise.all(data.filter(item => item.shouldVisualise).map(item => getNeighbourNode(item.concept, item.graqlVar, item.explanation, item.queryPattern))))
const nodes = (await Promise.all(data.filter(item => item.shouldVisualise).map(item => getNeighbourNode(item.concept, item.graqlVar, item.explanation))))
.reduce(collect, []);

const nodeIds = nodes.map(node => node.id);
Expand Down Expand Up @@ -556,7 +553,7 @@ const buildRPInstances = async (answers, currentData, shouldLimit, graknTx) => {
player.type().then(type => type.label().then((playerLabel) => {
player.label = playerLabel;
const edge = getEdge(relation, player, edgeTypes.instance.RELATES, edgeLabel);
getInstanceNode(player, graqlVar, answer.explanation, answer.queryPattern).then((node) => {
getInstanceNode(player, graqlVar, answer.explanation).then((node) => {
edges.push(edge);
nodes.push(node);
const isLastRole = i === rpEntries.length - 1;
Expand Down
38 changes: 32 additions & 6 deletions test/helpers/mockedConcepts.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@ const methods = {
label: () => Promise.resolve('thing'),
},
},
rule: {
static: {
isRule: () => true,
},
local: {
label: () => 'rule',
},
remote: {
label: () => Promise.resolve('rule'),
},
},
type: {
static: {
isType: () => true,
Expand Down Expand Up @@ -145,14 +156,20 @@ const methods = {
},
};

const getExtraProps = (mockerOptions, agentDefinedProps) => {
/**
* produces the props that will complete (or override) the defaults
* @param {*} userOptions // options provided by the end user (the test)
* @param {*} helperProps // options provided by the helper
* @returns acummilation of the given props, with userProps being superior
*/
const getExtraProps = (userOptions, helperProps) => {
const extraProps = { remote: {}, local: {} };

if (agentDefinedProps && agentDefinedProps.remote) extraProps.remote = { ...extraProps.remote, ...agentDefinedProps.remote };
if (mockerOptions && mockerOptions.extraProps && mockerOptions.extraProps.remote) extraProps.remote = { ...extraProps.remote, ...mockerOptions.extraProps.remote };
if (helperProps && helperProps.remote) extraProps.remote = { ...extraProps.remote, ...helperProps.remote };
if (userOptions && userOptions.extraProps && userOptions.extraProps.remote) extraProps.remote = { ...extraProps.remote, ...userOptions.extraProps.remote };

if (agentDefinedProps && agentDefinedProps.local) extraProps.local = { ...extraProps, ...agentDefinedProps.local };
if (mockerOptions && mockerOptions.extraProps && mockerOptions.extraProps.local) extraProps.local = { ...extraProps.local, ...mockerOptions.extraProps.local };
if (helperProps && helperProps.local) extraProps.local = { ...extraProps, ...helperProps.local };
if (userOptions && userOptions.extraProps && userOptions.extraProps.local) extraProps.local = { ...extraProps.local, ...userOptions.extraProps.local };

return extraProps;
};
Expand Down Expand Up @@ -223,6 +240,16 @@ export const getMockedRelationType = (options) => {
);
};

export const getMockedRule = (options) => {
const extraProps = getExtraProps(options);

return getMockedConcept(
['concept', 'rule'],
extraProps,
options && options.isRemote,
);
};

export const getMockedRole = (options) => {
const extraProps = getExtraProps(options);

Expand Down Expand Up @@ -282,7 +309,6 @@ export const getMockedConceptMap = (concepts, vars = [], explanationAnswers) =>
map: () => map,
hasExplanation: () => !!explanationAnswers,
explanation: () => ({ getAnswers: () => explanationAnswers || [] }),
queryPattern: () => '',
};
return mock;
};
Expand Down
11 changes: 8 additions & 3 deletions test/unit/components/Visualiser/store/actions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
} from '@/components/Visualiser/VisualiserUtils';
import VisualiserCanvasEventsHandler from '@/components/Visualiser/VisualiserCanvasEventsHandler';
import MockConcepts from '../../../../helpers/MockConcepts';
import { getMockedRelation, getMockedTransaction } from '../../../../helpers/mockedConcepts';
import { getMockedRelation, getMockedTransaction, getMockedRule } from '../../../../helpers/mockedConcepts';

jest.mock('@/components/Visualiser/VisualiserGraphBuilder', () => ({
prepareNodes: jest.fn(),
Expand Down Expand Up @@ -329,8 +329,13 @@ describe('actions', () => {

test("EXPLAIN_CONCEPT with unassigned relations in rule's when body", async () => {
const explanation = () => Promise.resolve({
getRule: () => Promise.resolve({
getWhen: () => Promise.resolve('{ $r ($a) isa some-relation; ($b) isa another-relation; }'),

getRule: () => getMockedRule({ isRemote: true,
extraProps: {
remote: {
getWhen: () => Promise.resolve('{ $r ($a) isa some-relation; ($b) isa another-relation; }'),
},
},
}),
});
const relation = getMockedRelation({
Expand Down
1 change: 0 additions & 1 deletion test/unit/components/shared/CanvasDataBuilder.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ const expectCommonPropsOnInstanceNode = (node) => {
expect(node).toHaveProperty('isInferred');
if (node.isInferred) {
expect(node).toHaveProperty('explanation');
expect(node).toHaveProperty('queryPattern');
}
expect(node).toHaveProperty('attributes');
};
Expand Down

0 comments on commit 458ca22

Please sign in to comment.