Skip to content

Commit 13f267c

Browse files
sblivensbliven
committed
Add GlobalsHelper class to manage our global state
Use GlobalsHelper.pushState()/restoreState() before and after tests to ensure that state isn't carried between tests. This is applied to the AtomCacheTest to fix test regressions while simplifying the code.
1 parent f5b325f commit 13f267c

File tree

4 files changed

+132
-28
lines changed

4 files changed

+132
-28
lines changed

‎biojava-structure/src/main/java/org/biojava/nbio/structure/StructureIO.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,11 @@ private static void checkInitAtomCache() {
119119
public static void setAtomCache(AtomCache c){
120120
cache = c;
121121
}
122+
123+
public static AtomCache getAtomCache() {
124+
checkInitAtomCache();
125+
return cache;
126+
}
122127

123128
/**
124129
* Returns the first biologicalAssembly that is available for a protein structure. For more documentation on quaternary structures see:

‎biojava-structure/src/main/java/org/biojava/nbio/structure/io/mmcif/DownloadChemCompProvider.java‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -95,25 +95,28 @@ public class DownloadChemCompProvider implements ChemCompProvider {
9595
boolean downloadAll = false;
9696

9797
public DownloadChemCompProvider(){
98-
logger.debug("Initialising DownloadChemCompProvider");
99-
100-
// note that path is static, so this is just to make sure that all non-static methods will have path initialised
101-
initPath();
98+
this(null);
10299
}
103100

104101
public DownloadChemCompProvider(String cacheFilePath){
105102
logger.debug("Initialising DownloadChemCompProvider");
106103

107104
// note that path is static, so this is just to make sure that all non-static methods will have path initialised
108-
path = new File(cacheFilePath);
105+
if(cacheFilePath != null) {
106+
path = new File(cacheFilePath);
107+
}
109108
}
110109

111-
private static void initPath(){
112-
110+
/**
111+
* Get this provider's cache path
112+
* @return
113+
*/
114+
public static File getPath(){
113115
if (path==null) {
114116
UserConfiguration config = new UserConfiguration();
115117
path = new File(config.getCacheFilePath());
116118
}
119+
return path;
117120
}
118121

119122
/**
@@ -130,7 +133,7 @@ public void checkDoFirstInstall(){
130133

131134
// this makes sure there is a file separator between every component,
132135
// if path has a trailing file separator or not, it will work for both cases
133-
File dir = new File(path, CHEM_COMP_CACHE_DIRECTORY);
136+
File dir = new File(getPath(), CHEM_COMP_CACHE_DIRECTORY);
134137
File f = new File(dir, "components.cif.gz");
135138

136139
if ( ! f.exists()) {
@@ -164,7 +167,7 @@ private void split() throws IOException {
164167

165168
logger.info("Installing individual chem comp files ...");
166169

167-
File dir = new File(path, CHEM_COMP_CACHE_DIRECTORY);
170+
File dir = new File(getPath(), CHEM_COMP_CACHE_DIRECTORY);
168171
File f = new File(dir, "components.cif.gz");
169172

170173

@@ -215,7 +218,7 @@ private void split() throws IOException {
215218
*/
216219
private void writeID(String contents, String currentID) throws IOException{
217220

218-
String localName = DownloadChemCompProvider.getLocalFileName(currentID);
221+
String localName = getLocalFileName(currentID);
219222

220223
try ( PrintWriter pw = new PrintWriter(new GZIPOutputStream(new FileOutputStream(localName))) ) {
221224

@@ -322,16 +325,15 @@ public static String getLocalFileName(String recordName){
322325
recordName = "_" + recordName;
323326
}
324327

325-
initPath();
326-
327-
File f = new File(path, CHEM_COMP_CACHE_DIRECTORY);
328+
File f = new File(getPath(), CHEM_COMP_CACHE_DIRECTORY);
328329
if (! f.exists()){
329330
logger.info("Creating directory " + f);
330331

331332
boolean success = f.mkdir();
332333
// we've checked in initPath that path is writable, so there's no need to check if it succeeds
333334
// in the unlikely case that in the meantime it isn't writable at least we log an error
334-
if (!success) logger.error("Directory {} could not be created",f);
335+
if (!success)
336+
logger.error("Directory {} could not be created",f);
335337

336338
}
337339

‎biojava-structure/src/test/java/org/biojava/nbio/structure/align/util/AtomCacheTest.java‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,12 @@
5555
import org.biojava.nbio.structure.io.LocalPDBDirectory.ObsoleteBehavior;
5656
import org.biojava.nbio.structure.io.MMCIFFileReader;
5757
import org.biojava.nbio.structure.io.mmcif.ChemCompGroupFactory;
58-
import org.biojava.nbio.structure.io.mmcif.ChemCompProvider;
5958
import org.biojava.nbio.structure.io.mmcif.DownloadChemCompProvider;
6059
import org.biojava.nbio.structure.io.mmcif.model.ChemComp;
6160
import org.biojava.nbio.structure.io.util.FileDownloadUtils;
6261
import org.biojava.nbio.structure.scop.ScopDatabase;
6362
import org.biojava.nbio.structure.scop.ScopFactory;
63+
import org.biojava.nbio.structure.test.util.GlobalsHelper;
6464
import org.junit.After;
6565
import org.junit.Before;
6666
import org.junit.Test;
@@ -77,32 +77,22 @@ public class AtomCacheTest {
7777

7878
private static Logger logger = LoggerFactory.getLogger(AtomCacheTest.class);
7979
private AtomCache cache;
80-
private String previousPDB_DIR;
81-
private String previousPDB_CACHE_DIR;
82-
private AtomCache cleanCache = new AtomCache();
83-
private ChemCompProvider previousChemCompProvider = ChemCompGroupFactory.getChemCompProvider();
8480

8581
@Before
8682
public void setUp() {
87-
previousPDB_DIR = System.getProperty(UserConfiguration.PDB_DIR, null);
88-
previousPDB_CACHE_DIR = System.getProperty(UserConfiguration.PDB_CACHE_DIR, null);
83+
GlobalsHelper.pushState();
84+
8985
cache = new AtomCache();
9086
cache.setObsoleteBehavior(ObsoleteBehavior.FETCH_OBSOLETE);
9187
StructureIO.setAtomCache(cache);
9288

9389
// Use a fixed SCOP version for stability
9490
ScopFactory.setScopDatabase(ScopFactory.VERSION_1_75B);
95-
logger.warn("setUp()");
9691
}
9792

9893
@After
9994
public void tearDown() {
100-
if (previousPDB_DIR != null) {
101-
System.setProperty(UserConfiguration.PDB_DIR, previousPDB_DIR);
102-
System.setProperty(UserConfiguration.PDB_CACHE_DIR, previousPDB_CACHE_DIR);
103-
}
104-
StructureIO.setAtomCache(cleanCache);
105-
ChemCompGroupFactory.setChemCompProvider(previousChemCompProvider);
95+
GlobalsHelper.restoreState();
10696
}
10797

10898
/**

‎biojava-structure/src/test/java/org/biojava/nbio/structure/test/util/GlobalsHelper.java‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
package org.biojava.nbio.structure.test.util;
2+
3+
import java.util.Deque;
4+
import java.util.LinkedList;
5+
import java.util.List;
6+
import java.util.NoSuchElementException;
7+
8+
import org.biojava.nbio.structure.StructureIO;
9+
import org.biojava.nbio.structure.align.util.AtomCache;
10+
import org.biojava.nbio.structure.align.util.UserConfiguration;
11+
import org.biojava.nbio.structure.io.mmcif.ChemCompGroupFactory;
12+
import org.biojava.nbio.structure.io.mmcif.ChemCompProvider;
13+
import org.biojava.nbio.structure.io.mmcif.DownloadChemCompProvider;
14+
import org.biojava.nbio.structure.scop.ScopDatabase;
15+
import org.biojava.nbio.structure.scop.ScopFactory;
16+
17+
/**
18+
* Helper class to manage all the global state changes in BioJava.
19+
* For instance, this should be used in tests before modifying PDB_PATH.
20+
*
21+
* Used by tests during setup and teardown to ensure a clean environment
22+
*
23+
* This class is a singleton.
24+
* @author Spencer Bliven
25+
*
26+
*/
27+
public final class GlobalsHelper {
28+
29+
private static class PathInfo {
30+
public final String pdbPath;
31+
public final String pdbCachePath;
32+
public final AtomCache atomCache;
33+
public final ChemCompProvider chemCompProvider;
34+
public final String downloadChemCompProviderPath;
35+
public final ScopDatabase scop;
36+
37+
public PathInfo() {
38+
pdbPath = System.getProperty(UserConfiguration.PDB_DIR, null);
39+
pdbCachePath = System.getProperty(UserConfiguration.PDB_CACHE_DIR, null);
40+
atomCache = StructureIO.getAtomCache();
41+
chemCompProvider = ChemCompGroupFactory.getChemCompProvider();
42+
downloadChemCompProviderPath = DownloadChemCompProvider.getPath().getPath();
43+
scop = ScopFactory.getSCOP();
44+
}
45+
}
46+
47+
// Saves defaults as stack
48+
private static Deque<PathInfo> stack = new LinkedList<>();
49+
static {
50+
// Save default state
51+
pushState();
52+
}
53+
54+
/**
55+
* GlobalsHelper should not be instantiated.
56+
*/
57+
private GlobalsHelper() {}
58+
59+
/**
60+
* Save current global state to the stack
61+
*/
62+
public static void pushState() {
63+
PathInfo paths = new PathInfo();
64+
stack.addFirst(paths);
65+
}
66+
67+
/**
68+
* Sets a new PDB_PATH and PDB_CACHE_PATH consistently.
69+
*
70+
* Previous values can be restored with {@link #restoreState()}.
71+
* @param path
72+
*/
73+
public static void setPdbPath(String path) {
74+
pushState();
75+
System.setProperty(UserConfiguration.PDB_DIR, path);
76+
System.setProperty(UserConfiguration.PDB_CACHE_DIR, path);
77+
78+
AtomCache cache = new AtomCache(path);
79+
StructureIO.setAtomCache(cache);
80+
81+
// Note side effect setting the path for all DownloadChemCompProvider due to static state
82+
ChemCompProvider provider = new DownloadChemCompProvider(path);
83+
ChemCompGroupFactory.setChemCompProvider(provider);
84+
}
85+
86+
/**
87+
* Restore global state to the previous settings
88+
* @throws NoSuchElementException if there is no prior state to restore
89+
*/
90+
public static void restoreState() {
91+
PathInfo paths = stack.removeFirst();
92+
93+
System.setProperty(UserConfiguration.PDB_DIR, paths.pdbPath);
94+
System.setProperty(UserConfiguration.PDB_CACHE_DIR, paths.pdbCachePath);
95+
96+
StructureIO.setAtomCache(paths.atomCache);
97+
98+
// Use side effect setting the path for all DownloadChemCompProvider due to static state
99+
new DownloadChemCompProvider(paths.downloadChemCompProviderPath);
100+
101+
ChemCompGroupFactory.setChemCompProvider(paths.chemCompProvider);
102+
103+
ScopFactory.setScopDatabase(paths.scop);
104+
}
105+
106+
107+
}

0 commit comments

Comments
 (0)