Skip to content

Commit

Permalink
Merge pull request #1318 from BladeRunnerJS/out-of-scope-exception-12…
Browse files Browse the repository at this point in the history
…32+565

OutOfScopeExceptions that list the scope and stricter enforcement so Blades cant depend on Blades in the Aspect scope
  • Loading branch information
ccpetercc committed Apr 10, 2015
2 parents 7475a34 + 4c25c40 commit c7e3cae
Show file tree
Hide file tree
Showing 24 changed files with 373 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
import org.bladerunnerjs.api.App;
import org.bladerunnerjs.api.Aspect;
import org.bladerunnerjs.api.model.exception.CircularDependencyException;
import org.bladerunnerjs.api.model.exception.OutOfBundleScopeRequirePathException;
import org.bladerunnerjs.api.model.exception.UnresolvableRelativeRequirePathException;
import org.bladerunnerjs.api.model.exception.UnresolvableRequirePathException;
import org.bladerunnerjs.api.model.exception.request.ContentProcessingException;
import org.bladerunnerjs.api.spec.engine.SpecTest;
import org.junit.Before;
Expand Down Expand Up @@ -72,7 +72,7 @@ public void weThrowAnExceptionIfARequiredCommonJsClassIsOnlyAvailableInAnotherAs
.and(aspect).classRequires("appns/Class1", "appns/Class2")
.and(aspect).indexPageRefersTo("appns.Class1");
when(aspect).requestReceivedInDev("js/dev/combined/bundle.js", response);
then(exceptions).verifyException(UnresolvableRequirePathException.class, "appns/Class2");
then(exceptions).verifyException(OutOfBundleScopeRequirePathException.class, "appns/Class2");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
import org.bladerunnerjs.api.Aspect;
import org.bladerunnerjs.api.Blade;
import org.bladerunnerjs.api.Bladeset;
import org.bladerunnerjs.api.model.exception.OutOfScopeRequirePathException;
import org.bladerunnerjs.api.model.exception.UnresolvableRequirePathException;
import org.bladerunnerjs.api.model.exception.request.ContentProcessingException;
import org.bladerunnerjs.api.spec.engine.SpecTest;
import org.bladerunnerjs.utility.BundleSetBuilder;
import org.junit.Before;
import org.junit.Test;

Expand Down Expand Up @@ -266,4 +268,91 @@ public void bladeClassesInDefaultBladesetCanBeBundled() throws Exception {
then(response).containsCommonJsClasses("appns/b1/Blade1Class", "appns/b2/Blade2Class");
}

@Test
public void exceptionIsThrownIfBladeClassRequestsAResourceFromDefaultAspect() throws Exception {
given(aspect).indexPageRequires("appns/b1/Blade1Class")
.and(blade1InDefaultBladeset).classRequires("Blade1Class", "appns/b2/Blade2Class")
.and(blade2InDefaultBladeset).hasClass("Blade2Class");
when(aspect).requestReceivedInDev("js/dev/combined/bundle.js", response);
then(exceptions).verifyException(OutOfScopeRequirePathException.class,
"appns/b1/Blade1Class", "appns/b2/Blade2Class", "blades/b2/src/Blade2Class.js", Blade.class.getSimpleName(),
"brjs-apps/app1/blades/b1, brjs-apps/app1")
.whereTopLevelExceptionIs(ContentProcessingException.class);
}

@Test
public void exceptionIsNotThrownIfBladeClassRequestsAResourceFromDefaultAspect_andRequiringBladeAssetContainerHasNoScrictCheckingFile() throws Exception {
given(aspect).indexPageRequires("appns/b1/Blade1Class")
.and(blade1InDefaultBladeset).classRequires("Blade1Class", "appns/b2/Blade2Class")
.and(blade1InDefaultBladeset).containsEmptyFile("no-strict-checking")
.and(blade2InDefaultBladeset).hasClass("Blade2Class");
when(aspect).requestReceivedInDev("js/dev/combined/bundle.js", response);
then(response).containsCommonJsClasses("appns/b1/Blade1Class", "appns/b2/Blade2Class")
.and(exceptions).verifyNoOutstandingExceptions();
}

@Test
public void exceptionIsNotThrownIfBladeClassRequestsAResourceFromDefaultAspect_andRequiredBladeAssetContainerHasNoScrictCheckingFile() throws Exception {
given(aspect).indexPageRequires("appns/b1/Blade1Class")
.and(blade1InDefaultBladeset).classRequires("Blade1Class", "appns/b2/Blade2Class")
.and(blade2InDefaultBladeset).hasClass("Blade2Class")
.and(blade2InDefaultBladeset).containsEmptyFile("no-strict-checking");
when(aspect).requestReceivedInDev("js/dev/combined/bundle.js", response);
then(response).containsCommonJsClasses("appns/b1/Blade1Class", "appns/b2/Blade2Class")
.and(exceptions).verifyNoOutstandingExceptions();
}

@Test
public void exceptionIsThrownIfBladeRequiresAnAspectClass() throws Exception {
given(aspect).indexPageRequires("appns/b1/Blade1Class")
.and(blade1InDefaultBladeset).classRequires("Blade1Class", "appns/AspectClass")
.and(aspect).hasClass("appns/AspectClass");
when(aspect).requestReceivedInDev("js/dev/combined/bundle.js", response);
then(exceptions).verifyException(OutOfScopeRequirePathException.class);
}

@Test
public void exceptionIsThrownIfBladeRequiresADefaultAspectClass() throws Exception {
given( app.defaultAspect() ).indexPageRequires("appns/b1/Blade1Class")
.and(blade1InDefaultBladeset).classRequires("Blade1Class", "appns/AspectClass")
.and( app.defaultAspect() ).hasClass("appns/AspectClass");
when( app.defaultAspect() ).requestReceivedInDev("js/dev/combined/bundle.js", response);
then(exceptions).verifyException(OutOfScopeRequirePathException.class);
}

@Test
public void noStrictCheckingFileCanBeAtANestedLevelInsideTheBlade() throws Exception {
given(aspect).indexPageRequires("appns/b1/Blade1Class")
.and(blade1InDefaultBladeset).classRequires("Blade1Class", "appns/b2/foo/Blade2Class")
.and(blade2InDefaultBladeset).hasDir("src/foo")
.and(blade2InDefaultBladeset).containsEmptyFile("src/foo/no-strict-checking")
.and(blade2InDefaultBladeset).hasClass("foo/Blade2Class");
when(aspect).requestReceivedInDev("js/dev/combined/bundle.js", response);
then(response).containsCommonJsClasses("appns/b1/Blade1Class", "appns/b2/Blade2Class")
.and(exceptions).verifyNoOutstandingExceptions();
}

@Test
public void noStrictCheckingFileCanBeAtANestedLevelInsideTheBladeAndOnlyAppliesToSubfolders() throws Exception {
given(aspect).indexPageRequires("appns/b1/Blade1Class")
.and(blade1InDefaultBladeset).classRequires("Blade1Class", "appns/b2/Blade2Class")
.and(blade2InDefaultBladeset).hasDir("src/foo")
.and(blade2InDefaultBladeset).containsEmptyFile("src/foo/no-strict-checking")
.and(blade2InDefaultBladeset).hasClass("Blade2Class");
when(aspect).requestReceivedInDev("js/dev/combined/bundle.js", response);
then(exceptions).verifyException(OutOfScopeRequirePathException.class);
}

@Test
public void warningIsLoggedWhenStrictCheckingIsDisabled() throws Exception {
given(aspect).indexPageRequires("appns/b1/Blade1Class")
.and(blade1InDefaultBladeset).classRequires("Blade1Class", "appns/b2/foo/Blade2Class")
.and(blade2InDefaultBladeset).hasDir("src/foo")
.and(blade2InDefaultBladeset).containsEmptyFile("src/foo/no-strict-checking")
.and(blade2InDefaultBladeset).hasClass("foo/Blade2Class")
.and(logging).enabled();
when(aspect).requestReceivedInDev("js/dev/combined/bundle.js", response);
then(logging).warnMessageReceived(BundleSetBuilder.STRICT_CHECKING_DISABLED_MSG, "brjs-apps/app1/blades/b2/src/foo", "blades/b2/src/foo/Blade2Class.js", "brjs-apps/app1/blades/b2/src/foo/no-strict-checking");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import org.bladerunnerjs.api.Blade;
import org.bladerunnerjs.api.Bladeset;
import org.bladerunnerjs.api.JsLib;
import org.bladerunnerjs.api.model.exception.OutOfBundleScopeRequirePathException;
import org.bladerunnerjs.api.model.exception.request.ContentProcessingException;
import org.bladerunnerjs.api.spec.engine.SpecTest;
import org.bladerunnerjs.model.NamedDirNode;
import org.bladerunnerjs.api.BladeWorkbench;
Expand Down Expand Up @@ -119,6 +121,29 @@ public void weBundleBootstrapBeforeOtherLibsFromTheApp() throws Exception {
then(response).containsOrderedTextFragments("// br-bootstrap",
"// appLib" );
}

@Test
public void outOfScopeExceptionIsThrownIfTheRequiredClassIsOutOfScope() throws Exception {
given(aspect).hasClass("appns/App")
.and(blade).classRequires("appns/bs/b1/Class1", "appns/App")
.and(blade.workbench()).indexPageRequires("appns/bs/b1/Class1");
when(blade.workbench()).requestReceivedInDev("js/dev/combined/bundle.js", response);
then(exceptions).verifyException(OutOfBundleScopeRequirePathException.class,
"appns/App", "default-aspect/src/appns/App.js", BladeWorkbench.class.getSimpleName(),
"brjs-apps/app1/bs-bladeset, brjs-apps/app1/bs-bladeset/blades/b1, brjs-apps/app1/bs-bladeset/blades/b1/workbench")
.whereTopLevelExceptionIs(ContentProcessingException.class);
}

@Test
public void outOfScopeExceptionContainsTheFileWithTheException() throws Exception {
given(aspect).hasClass("appns/App")
.and(blade).classRequires("appns/bs/b1/Class1", "appns/App")
.and(blade.workbench()).indexPageRequires("appns/bs/b1/Class1");
when(blade.workbench()).requestReceivedInDev("js/dev/combined/bundle.js", response);
then(exceptions).verifyException(OutOfBundleScopeRequirePathException.class,
"bs-bladeset/blades/b1/src/appns/bs/b1/Class1.js")
.whereTopLevelExceptionIs(ContentProcessingException.class);
}


// ----------------------------------- C S S -------------------------------------
Expand Down
1 change: 1 addition & 0 deletions brjs-core/src/main/java/org/bladerunnerjs/api/Asset.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ public interface Asset {
List<String> getRequirePaths();
String getPrimaryRequirePath();
AssetContainer assetContainer();
boolean isScopeEnforced();
boolean isRequirable();
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,14 @@ public ModelOperationException(String message) {
public ModelOperationException(String message, Exception e) {
super(message + "; " + e.getMessage(), e);
}

@Override
public String getMessage()
{
if (getCause() != null) {
// the Java Throwable class caches the exception message, override it and call through to the cause so the cause message can change as it gets given more information up the call stack
return getCause().getMessage();
}
return super.getMessage();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package org.bladerunnerjs.api.model.exception;

import org.bladerunnerjs.api.Asset;
import org.bladerunnerjs.api.BundlableNode;
import org.bladerunnerjs.api.LinkedAsset;

public class OutOfBundleScopeRequirePathException extends RequirePathException {

private static final long serialVersionUID = 1L;
private String requirePath;
private String scopedLocations;
private Asset asset;
private BundlableNode bundlableNode;
private Asset assetWithException = null;

public OutOfBundleScopeRequirePathException(BundlableNode bundlableNode, String requirePath, Asset asset) {
this.bundlableNode = bundlableNode;
this.requirePath = requirePath;
this.asset = asset;
scopedLocations = RequirePathExceptionUtils.getScopeLocationText(bundlableNode);
}

public void setAssetWithException(LinkedAsset asset) {
assetWithException = asset;
}

@Override
public String getMessage() {
if (assetWithException != null) {
return String.format("There was an exception calculating dependencies for the asset at '%s'. It's dependency with the require path '%s' was found at '%s', but it was not in one of the valid bundler scopes."+
" The bundlable node was '%s' and the valid locations for assets in this scope are '%s'",
assetWithException.getAssetPath(), requirePath, asset.getAssetPath(), bundlableNode.getClass().getSimpleName(), scopedLocations);
}
return String.format("The asset with the require path '%s' was found at '%s', but it was not in one of the valid bundler scopes."+
" The bundlable node was '%s' and the valid locations for assets in this scope are '%s'",
requirePath, asset.getAssetPath(), bundlableNode.getClass().getSimpleName(), scopedLocations);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package org.bladerunnerjs.api.model.exception;

import org.bladerunnerjs.api.Asset;
import org.bladerunnerjs.api.LinkedAsset;


public class OutOfScopeRequirePathException extends RequirePathException
{
private static final long serialVersionUID = 4008632015949516509L;
private LinkedAsset sourceAsset;
private Asset dependantAsset;
private String scopedLocations;

public OutOfScopeRequirePathException(LinkedAsset sourceAsset, Asset dependantAsset) {
this.sourceAsset = sourceAsset;
this.dependantAsset = dependantAsset;
scopedLocations = RequirePathExceptionUtils.getScopeLocationText(sourceAsset.assetContainer());
}

@Override
public String getMessage()
{
return String.format(
"The asset with the primary require path '%s' has a dependency on the asset with the primary require path '%s',"+
" which is located at '%s' and is outside of the assets' scope."+
" The source asset is contained within the '%s' scope and can only depend on the assets in the following locations: '%s'.",
sourceAsset.getPrimaryRequirePath(), dependantAsset.getPrimaryRequirePath(), dependantAsset.getAssetPath(),
sourceAsset.assetContainer().getClass().getSimpleName(), scopedLocations);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package org.bladerunnerjs.api.model.exception;

import org.bladerunnerjs.api.BRJS;
import org.bladerunnerjs.model.AssetContainer;


public class RequirePathExceptionUtils
{

public static String getScopeLocationText(AssetContainer assetContainer) {
BRJS brjs = assetContainer.root();
StringBuilder scopedLocationsBuilder = new StringBuilder();
for (AssetContainer scopeAssetContainer : assetContainer.scopeAssetContainers()) {
if (scopedLocationsBuilder.length() > 0) {
scopedLocationsBuilder.append(", ");
}
scopedLocationsBuilder.append( brjs.dir().getRelativePath(scopeAssetContainer.dir()) );
}
return scopedLocationsBuilder.toString();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ public AssetContainer assetContainer()
return assetContainer;
}

@Override
public boolean isScopeEnforced() {
return true;
}

@Override
public boolean isRequirable()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,10 @@ public static String calculateRequirePath(String requirePrefix, MemoizedFile ass
return requirePrefix+"/"+assetFile.requirePathName();
}

@Override
public boolean isScopeEnforced()
{
return true;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,11 @@ public AssetContainer assetContainer()
return assetContainer;
}

@Override
public boolean isScopeEnforced() {
return true;
}

@Override
public boolean isRequirable()
{
Expand Down
Loading

0 comments on commit c7e3cae

Please sign in to comment.