Skip to content

Commit

Permalink
[FIX] XMLTemplateAnalyzer: Properly detect conditional dependencies
Browse files Browse the repository at this point in the history
The conditional flag is now properly taken into account when adding
dependencies within tempate:if or template:repeat tags.

The async execution of ResourcePool#findResource caused the
'conditional' flag to be incorrect as it is updated during the sync
traversal of the nodes.

JIRA: CPOUI5FOUNDATION-347
  • Loading branch information
matz3 committed May 17, 2021
1 parent e27d10f commit 0f56490
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 68 deletions.
138 changes: 70 additions & 68 deletions lib/lbt/analyzer/XMLTemplateAnalyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,9 @@ class XMLTemplateAnalyzer {
* Add a dependency if it is new.
*
* @param {string} moduleName
* @param {boolean} conditional
*/
_addDependency(moduleName) {
_addDependency(moduleName, conditional) {
// don't add references to 'self'
if ( this.info.name === moduleName ) {
return;
Expand All @@ -117,7 +118,7 @@ class XMLTemplateAnalyzer {
return;
}

this.info.addDependency(moduleName, this.conditional);
this.info.addDependency(moduleName, conditional);
}

/**
Expand Down Expand Up @@ -194,15 +195,15 @@ class XMLTemplateAnalyzer {

const controllerName = getAttribute(node, XMLVIEW_CONTROLLERNAME_ATTRIBUTE);
if ( controllerName ) {
this._addDependency( ModuleName.fromUI5LegacyName(controllerName, ".controller.js") );
this._addDependency( ModuleName.fromUI5LegacyName(controllerName, ".controller.js"), this.conditional );
}

const resourceBundleName = getAttribute(node, XMLVIEW_RESBUNDLENAME_ATTRIBUTE);
if ( resourceBundleName ) {
const resourceBundleModuleName = ModuleName.fromUI5LegacyName(resourceBundleName, ".properties");
log.verbose("found dependency to resource bundle %s", resourceBundleModuleName);
// TODO locale dependent dependencies: this._addDependency(resourceBundleModuleName);
this._addDependency( resourceBundleModuleName );
this._addDependency( resourceBundleModuleName, this.conditional );
}

this._analyzeCoreRequire(node);
Expand Down Expand Up @@ -233,69 +234,7 @@ class XMLTemplateAnalyzer {
// ignore FragmentDefinition (also skipped by runtime XMLTemplateProcessor)
if ( FRAGMENTDEFINITION_MODULE !== moduleName ) {
this._analyzeCoreRequire(node);

this.promises.push(

this._pool.findResource(moduleName).then( () => {
this._addDependency(moduleName);

// handle special controls that reference other entities via name
// - (HTML|JS|JSON|XML)View reference another view by 'viewName'
// - ComponentContainer reference another component by 'componentName'
// - Fragment references a fragment by 'fragmentName' . 'type'

if ( moduleName === COMPONENTCONTAINER_MODULE ) {
const componentName = getAttribute(node, COMPONENTCONTAINER_COMPONENTNAME_ATTRIBUTE);
if ( componentName ) {
const componentModuleName =
ModuleName.fromUI5LegacyName( componentName, "/Component.js" );
this._addDependency(componentModuleName);
}
// TODO what about component.json? handle it transitively via Component.js?
} else if ( moduleName === FRAGMENT_MODULE ) {
const fragmentName = getAttribute(node, FRAGMENT_FRAGMENTNAME_ATTRIBUTE);
const type = getAttribute(node, FRAGMENT_TYPE_ATTRIBUTE);
if ( fragmentName && type ) {
const fragmentModuleName =
ModuleName.fromUI5LegacyName( fragmentName, this._getFragmentExtension(type) );
// console.log("child fragment detected %s", fragmentModuleName);
this._addDependency(fragmentModuleName);
}
} else if ( moduleName === HTMLVIEW_MODULE ) {
const viewName = getAttribute(node, ANYVIEW_VIEWNAME_ATTRIBUTE);
if ( viewName ) {
const childViewModuleName = ModuleName.fromUI5LegacyName( viewName, ".view.html" );
// console.log("child view detected %s", childViewModuleName);
this._addDependency(childViewModuleName);
}
} else if ( moduleName === JSVIEW_MODULE ) {
const viewName = getAttribute(node, ANYVIEW_VIEWNAME_ATTRIBUTE);
if ( viewName ) {
const childViewModuleName = ModuleName.fromUI5LegacyName( viewName, ".view.js" );
// console.log("child view detected %s", childViewModuleName);
this._addDependency(childViewModuleName);
}
} else if ( moduleName === JSONVIEW_MODULE ) {
const viewName = getAttribute(node, ANYVIEW_VIEWNAME_ATTRIBUTE);
if ( viewName ) {
const childViewModuleName = ModuleName.fromUI5LegacyName( viewName, ".view.json" );
// console.log("child view detected %s", childViewModuleName);
this._addDependency(childViewModuleName);
}
} else if ( moduleName === XMLVIEW_MODULE ) {
const viewName = getAttribute(node, ANYVIEW_VIEWNAME_ATTRIBUTE);
if ( viewName ) {
const childViewModuleName = ModuleName.fromUI5LegacyName( viewName, ".view.xml" );
// console.log("child view detected %s", childViewModuleName);
this._addDependency(childViewModuleName);
}
}
}, (err) => {
// ignore missing resources
// console.warn( "node not found %s", moduleName);
})

);
this.promises.push(this._analyzeModuleDependency(node, moduleName, this.conditional));
}
}

Expand Down Expand Up @@ -343,7 +282,7 @@ class XMLTemplateAnalyzer {
Object.keys(requireContext).forEach((key) => {
const requireJsName = requireContext[key];
if ( requireJsName && typeof requireJsName === "string" ) {
this._addDependency(ModuleName.fromRequireJSName(requireJsName));
this._addDependency(ModuleName.fromRequireJSName(requireJsName), this.conditional);
} else {
log.error(`Ignoring core:require: '${key}' refers to invalid module name '${requireJsName}'`);
}
Expand All @@ -352,6 +291,69 @@ class XMLTemplateAnalyzer {
}
}

async _analyzeModuleDependency(node, moduleName, conditional) {
try {
await this._pool.findResource(moduleName);

this._addDependency(moduleName, conditional);

// handle special controls that reference other entities via name
// - (HTML|JS|JSON|XML)View reference another view by 'viewName'
// - ComponentContainer reference another component by 'componentName'
// - Fragment references a fragment by 'fragmentName' . 'type'

if ( moduleName === COMPONENTCONTAINER_MODULE ) {
const componentName = getAttribute(node, COMPONENTCONTAINER_COMPONENTNAME_ATTRIBUTE);
if ( componentName ) {
const componentModuleName =
ModuleName.fromUI5LegacyName( componentName, "/Component.js" );
this._addDependency(componentModuleName, conditional);
}
// TODO what about component.json? handle it transitively via Component.js?
} else if ( moduleName === FRAGMENT_MODULE ) {
const fragmentName = getAttribute(node, FRAGMENT_FRAGMENTNAME_ATTRIBUTE);
const type = getAttribute(node, FRAGMENT_TYPE_ATTRIBUTE);
if ( fragmentName && type ) {
const fragmentModuleName =
ModuleName.fromUI5LegacyName( fragmentName, this._getFragmentExtension(type) );
// console.log("child fragment detected %s", fragmentModuleName);
this._addDependency(fragmentModuleName, conditional);
}
} else if ( moduleName === HTMLVIEW_MODULE ) {
const viewName = getAttribute(node, ANYVIEW_VIEWNAME_ATTRIBUTE);
if ( viewName ) {
const childViewModuleName = ModuleName.fromUI5LegacyName( viewName, ".view.html" );
// console.log("child view detected %s", childViewModuleName);
this._addDependency(childViewModuleName, conditional);
}
} else if ( moduleName === JSVIEW_MODULE ) {
const viewName = getAttribute(node, ANYVIEW_VIEWNAME_ATTRIBUTE);
if ( viewName ) {
const childViewModuleName = ModuleName.fromUI5LegacyName( viewName, ".view.js" );
// console.log("child view detected %s", childViewModuleName);
this._addDependency(childViewModuleName, conditional);
}
} else if ( moduleName === JSONVIEW_MODULE ) {
const viewName = getAttribute(node, ANYVIEW_VIEWNAME_ATTRIBUTE);
if ( viewName ) {
const childViewModuleName = ModuleName.fromUI5LegacyName( viewName, ".view.json" );
// console.log("child view detected %s", childViewModuleName);
this._addDependency(childViewModuleName, conditional);
}
} else if ( moduleName === XMLVIEW_MODULE ) {
const viewName = getAttribute(node, ANYVIEW_VIEWNAME_ATTRIBUTE);
if ( viewName ) {
const childViewModuleName = ModuleName.fromUI5LegacyName( viewName, ".view.xml" );
// console.log("child view detected %s", childViewModuleName);
this._addDependency(childViewModuleName, conditional);
}
}
} catch (err) {
// ignore missing resources
// console.warn( "node not found %s", moduleName);
}
}

_getFragmentExtension(type) {
return ".fragment." + type.toLowerCase();
}
Expand Down
8 changes: 8 additions & 0 deletions test/lib/lbt/analyzer/XMLTemplateAnalyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ test.serial("integration: Analysis of an xml view with core:require from databin
], "Dependencies should come from the XML template");
t.true(moduleInfo.isImplicitDependency("sap/ui/core/mvc/XMLView.js"),
"Implicit dependency should be added since an XMLView is analyzed");
t.true(
!moduleInfo.isConditionalDependency("sap/m/Button.js") &&
!moduleInfo.isImplicitDependency("sap/m/Button.js"),
"A control outside of template:if should become a strict dependency");

t.is(errorLogStub.callCount, 1, "should be called 1 time");
t.deepEqual(errorLogStub.getCall(0).args, [
Expand Down Expand Up @@ -156,6 +160,8 @@ test.serial("integration: Analysis of an xml view with core:require from databin
], "Dependencies should come from the XML template");
t.true(moduleInfo.isImplicitDependency("sap/ui/core/mvc/XMLView.js"),
"Implicit dependency should be added since an XMLView is analyzed");
t.true(moduleInfo.isConditionalDependency("sap/m/Button.js"),
"A control within template:if or template:repeat should become a conditional dependency");

t.is(verboseLogStub.callCount, 1, "should be called 1 time");
t.deepEqual(verboseLogStub.getCall(0).args, [
Expand Down Expand Up @@ -202,6 +208,8 @@ test.serial("integration: Analysis of an xml view with core:require from express
], "Dependencies should come from the XML template");
t.true(moduleInfo.isImplicitDependency("sap/ui/core/mvc/XMLView.js"),
"Implicit dependency should be added since an XMLView is analyzed");
t.true(moduleInfo.isConditionalDependency("sap/m/Button.js"),
"A control within template:if should become a conditional dependency");

t.is(verboseLogStub.callCount, 1, "should be called 1 time");
t.deepEqual(verboseLogStub.getCall(0).args, [
Expand Down

0 comments on commit 0f56490

Please sign in to comment.