Skip to content

Commit

Permalink
[resolve] Improvements after BJ's comments
Browse files Browse the repository at this point in the history 
- Uses the project/workspace lastmodifiedtime, which includes include files
- Removed removal of container errors in test, uses a resolve file that has proper dependencies
- Added a testReason so the caching choice could be tested from outside
- Increased test coverage



Signed-off-by: Peter Kriens <Peter.Kriens@aqute.biz>
  • Loading branch information
@pkriens
pkriens committed Jul 1, 2022
1 parent 9d82706 commit ceba753
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 57 deletions.
56 changes: 34 additions & 22 deletions biz.aQute.resolve/src/biz/aQute/resolve/Bndrun.java
View file
Expand Up @@ -15,6 +15,7 @@
import aQute.bnd.build.Container;
import aQute.bnd.build.Run;
import aQute.bnd.build.Workspace;
import aQute.bnd.build.WorkspaceLayout;
import aQute.bnd.build.model.BndEditModel;
import aQute.bnd.build.model.clauses.HeaderClause;
import aQute.bnd.build.model.clauses.VersionedClause;
Expand Down Expand Up @@ -194,38 +195,55 @@ public Collection<Container> getRunbundles() throws Exception {
return super.getRunbundles();
}

enum CacheReason {
NO_CACHE_FILE,
NOT_A_BND_LAYOUT,
CACHE_STALE_PROJECT,
CACHE_STALE_WORKSPACE,
USE_CACHE,
INVALID_CACHE;

}

CacheReason testReason;

private Collection<Container> cache() throws Exception {
Workspace ws = getWorkspace();
File ours = getPropertiesFile();
File cache = getCacheFile(ours);
File workspace = getWorkspace().getPropertiesFile();
if (workspace == null)
workspace = ours;

String force;
if (!cache.isFile())
force = "no cache file";
else if (cache.lastModified() <= ours.lastModified())
force = "cache file is older than our properties";
else if (cache.lastModified() <= workspace.lastModified())
force = "cache file is older than our workspace";

long cacheLastModified = cache.lastModified();

CacheReason reason;
if (ws.getLayout() != WorkspaceLayout.BND)
reason = CacheReason.NOT_A_BND_LAYOUT;
else if (!cache.isFile())
reason = CacheReason.NO_CACHE_FILE;
else if (cacheLastModified < ws.lastModified())
reason = CacheReason.CACHE_STALE_WORKSPACE;
else if (cacheLastModified < lastModified())
reason = CacheReason.CACHE_STALE_PROJECT;
else
force = null;
trace("force = %s", force);
reason = CacheReason.USE_CACHE;

boolean tryCache = force == null;
testReason = reason;

trace("force = %s", reason);

try (Processor p = new Processor()) {

if (cache.isFile())
p.setProperties(cache);

if (tryCache) {
if (reason == CacheReason.USE_CACHE) {
trace("attempting to use cache");
String runbundles = p.getProperty(Constants.RUNBUNDLES);
Collection<Container> containers = parseRunbundles(runbundles);
if (isAllOk(containers)) {
trace("from cache %s", containers);
return containers;
} else {
testReason = CacheReason.INVALID_CACHE;
trace("the cached bundles were not ok, will resolve");
}
}
Expand Down Expand Up @@ -255,6 +273,7 @@ else if (cache.lastModified() <= workspace.lastModified())
}
}


/**
* Return the file used to cache the resolved solution for the given file
*
Expand All @@ -274,16 +293,9 @@ public File getCacheFile(File file) {
}
}

boolean testIgnoreDownloadErrors = false;

private boolean isAllOk(Collection<Container> containers) {
for (Container c : containers) {
if (c.getError() != null) {
if (testIgnoreDownloadErrors) {
if (c.getError()
.contains("FileNotFoundException"))
continue;
}
return false;
}
}
Expand Down
141 changes: 106 additions & 35 deletions biz.aQute.resolve/test/biz/aQute/resolve/RunResolutionTest.java
View file
@@ -1,6 +1,7 @@
package biz.aQute.resolve;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import java.io.File;
Expand Down Expand Up @@ -28,6 +29,7 @@
import aQute.bnd.result.Result;
import aQute.bnd.test.jupiter.InjectTemporaryDirectory;
import aQute.lib.io.IO;
import biz.aQute.resolve.Bndrun.CacheReason;

public class RunResolutionTest {

Expand Down Expand Up @@ -121,45 +123,114 @@ public void testCachingOfResult() throws Exception {
}

@Test
public void testResolveCached() throws Exception {
public void testResolveCachedWithStandalone() throws Exception {
Bndrun bndrun = Bndrun.createBndrun(workspace, IO.getFile(tmp.toFile(), "resolver.bndrun"));
bndrun.setProperty("-resolve", "cache");
Collection<Container> runbundles = bndrun.getRunbundles();
assertThat(bndrun.testReason).isEqualTo(CacheReason.NOT_A_BND_LAYOUT);
}

Bndrun bndrun = Bndrun.createBndrun(workspace, IO.getFile(ws.toFile(), "test.simple/resolve.bndrun"));
@Test
public void testResolveCached() throws Exception {
Bndrun bndrun = Bndrun.createBndrun(workspace, workspace.getFile("test.simple/resolve.bndrun"));
bndrun.setTrace(true);
File file = bndrun.getPropertiesFile();
bndrun.testIgnoreDownloadErrors = true;
assertTrue(bndrun.check());
File cache = bndrun.getCacheFile(file);

System.out.println("get the embedded list of runbundles");
bndrun.setProperty("-resolve", "manual");
Collection<Container> manual = bndrun.getRunbundles();

System.out.println("remove the embedded list and set mode to 'cache'");
bndrun.setProperty("-resolve", "cache");
bndrun.unsetProperty("-runbundles");

assertThat(cache).doesNotExist();

System.out.println("First time we should resolve & create a cache file");
Collection<Container> cached = bndrun.getRunbundles();
assertThat(cache).isFile();
assertThat(cached).containsAll(manual);
assertThat(cached).hasSize(manual.size());
assertThat(cache.lastModified()).isGreaterThan(file.lastModified());

System.out
.println("Second time, the cache file should used, so make it valid but empty ");
long lastModified = cache.lastModified();
IO.store("-runbundles ", cache);
cache.setLastModified(file.lastModified() + 1000);
cached = bndrun.getRunbundles();
assertThat(cached).isEmpty();

System.out.println("Now make cache invalid, which be ignored");
IO.store("-runbundles is not a valid file", cache);
cache.setLastModified(file.lastModified() + 1000);
cached = bndrun.getRunbundles();
assertThat(manual).containsAll(cached);

File build = IO.getFile(ws.toFile(), "cnf/build.bnd");

try {

System.out.println("get the embedded list of runbundles, this is out benchmark");
bndrun.setProperty("-resolve", "manual");
Collection<Container> manual = bndrun.getRunbundles();
assertThat(manual).hasSize(2);

System.out.println("remove the embedded list and set mode to 'cache'");
bndrun.setProperty("-resolve", "cache");
bndrun.unsetProperty("-runbundles");

assertThat(cache).doesNotExist();

System.out.println("First time we should resolve & create a cache file");
Collection<Container> cached = bndrun.getRunbundles();
assertTrue(bndrun.check());
assertThat(cache).isFile();
assertThat(cached).containsExactlyElementsOf(manual);
assertThat(cache.lastModified()).isGreaterThan(bndrun.lastModified());
assertThat(bndrun.testReason).isEqualTo(CacheReason.NO_CACHE_FILE);

System.out.println("Second time, the cache file should used, so make it valid but empty ");
long lastModified = cache.lastModified();
IO.store("-runbundles ", cache);
cached = bndrun.getRunbundles();
assertTrue(bndrun.check());
assertThat(cached).isEmpty();
assertThat(bndrun.testReason).isEqualTo(CacheReason.USE_CACHE);

System.out.println("Now make cache invalid, should be ignored");
IO.store("-runbundles is not a valid file", cache);
cached = bndrun.getRunbundles();
assertTrue(bndrun.check());
assertThat(cached).containsExactlyElementsOf(manual);
assertThat(bndrun.testReason).isEqualTo(CacheReason.INVALID_CACHE);

System.out.println("Now empty cache, but still use it");
IO.store("-runbundles ", cache);
cached = bndrun.getRunbundles();
assertTrue(bndrun.check());
assertThat(cached).isEmpty();
assertThat(bndrun.testReason).isEqualTo(CacheReason.USE_CACHE);

System.out.println("Refresh and check we still use the cache");
assertFalse(bndrun.refresh());
bndrun.setProperty("-resolve", "cache");
bndrun.unsetProperty("-runbundles");
cached = bndrun.getRunbundles();
assertThat(cached).isEmpty();
assertThat(bndrun.testReason).isEqualTo(CacheReason.USE_CACHE);

System.out.println("Update an include file, refresh and check we still sue the cache");
File empty = IO.getFile(ws.toFile(), "test.simple/empty-included-in-resolve.bnd");
long now = System.currentTimeMillis();
empty.setLastModified(now);
assertThat(bndrun.lastModified()).isLessThan(now);
assertThat(cache.lastModified()).isLessThan(now);
assertTrue(bndrun.refresh());
bndrun.setProperty("-resolve", "cache");
bndrun.unsetProperty("-runbundles");
assertThat(bndrun.lastModified()).isGreaterThanOrEqualTo(now);
cached = bndrun.getRunbundles();
assertTrue(bndrun.check());
assertThat(cached).containsExactlyElementsOf(manual);
assertThat(bndrun.testReason).isEqualTo(CacheReason.CACHE_STALE_PROJECT);
assertThat(cache.lastModified()).isGreaterThanOrEqualTo(now);
assertThat(cache.lastModified()).isGreaterThanOrEqualTo(bndrun.lastModified());

System.out.println("Next we use the cache");
cached = bndrun.getRunbundles();
assertThat(cached).containsExactlyElementsOf(manual);
assertThat(bndrun.testReason).isEqualTo(CacheReason.USE_CACHE);

System.out.println("Update the cnf/build file");
now = System.currentTimeMillis();
build.setLastModified(now);
assertTrue(workspace.refresh());
cached = bndrun.getRunbundles();
assertThat(bndrun.testReason).isEqualTo(CacheReason.CACHE_STALE_WORKSPACE);

System.out.println("Next we use the cache again");
cached = bndrun.getRunbundles();
assertThat(bndrun.testReason).isEqualTo(CacheReason.USE_CACHE);

assertTrue(bndrun.check());
} catch (AssertionError e) {
System.out.println("bndrun = " + bndrun.lastModified());
System.out.println("cache = " + cache.lastModified());
System.out.println("workspace = " + workspace.lastModified());
System.out.println("build = " + build.lastModified());
throw e;
}
}

@Test
Expand Down
Empty file added biz.aQute.resolve/testdata/pre-buildworkspace/test.simple/empty-included-in-resolve.bnd
View file
Empty file.
1 change: 1 addition & 0 deletions biz.aQute.resolve/testdata/pre-buildworkspace/test.simple/resolve.bndrun
View file
@@ -1,3 +1,4 @@
-include empty-included-in-resolve.bnd
-runfw: org.eclipse.osgi;version='[3.13.0,3.13.1)'
-runee: JavaSE-1.8
-runrequires: osgi.identity;filter:='(osgi.identity=test.simple)'
Expand Down

0 comments on commit ceba753

Please sign in to comment.