Skip to content

Commit 40a1e8f

Browse files
committed
Merge pull request #405 from sbliven/master
Fix resource leaks with ChemComp
2 parents 1b3307a + 028eaaa commit 40a1e8f

File tree

5 files changed

+129
-142
lines changed

5 files changed

+129
-142
lines changed

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,8 @@ public static ChemComp getChemComp(String recordName){
5757
logger.debug("Chem comp "+recordName+" read from provider "+chemCompProvider.getClass().getCanonicalName());
5858
cc = chemCompProvider.getChemComp(recordName);
5959

60-
// If chemCompProvider fails don't try to cache null & one letter code may be null.
61-
if (null != cc && !"?".equals(cc.getOne_letter_code())){
62-
cache.put(recordName, cc);
63-
}
60+
// Note that this also caches null or empty responses
61+
cache.put(recordName, cc);
6462
return cc;
6563
}
6664

@@ -79,6 +77,8 @@ public static ChemComp getChemComp(String recordName){
7977
public static void setChemCompProvider(ChemCompProvider provider) {
8078
logger.debug("Setting new chem comp provider to "+provider.getClass().getCanonicalName());
8179
chemCompProvider = provider;
80+
// clear cache
81+
cache.clear();
8282
}
8383

8484
public static ChemCompProvider getChemCompProvider(){
@@ -154,9 +154,4 @@ public static String getOneLetterCode(ChemComp cc){
154154
return oneLetter;
155155
}
156156

157-
public static SoftHashMap<String, ChemComp> getCache() {
158-
return cache;
159-
}
160-
161-
162157
}

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

Lines changed: 43 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -166,62 +166,57 @@ private void split() throws IOException {
166166
int counter = 0;
167167
InputStreamProvider prov = new InputStreamProvider();
168168

169-
InputStream inStream = prov.getInputStream(f);
170-
171-
BufferedReader buf = new BufferedReader (new InputStreamReader (inStream));
169+
try( BufferedReader buf = new BufferedReader (new InputStreamReader (prov.getInputStream(f)));
170+
) {
171+
String line = null;
172+
line = buf.readLine ();
173+
StringWriter writer = new StringWriter();
172174

173-
String line = null;
174-
line = buf.readLine ();
175-
StringWriter writer = new StringWriter();
175+
String currentID = null;
176+
while (line != null){
176177

177-
String currentID = null;
178-
while (line != null){
178+
if ( line.startsWith("data_")) {
179+
// a new record found!
179180

180-
if ( line.startsWith("data_")) {
181-
// a new record found!
181+
if ( currentID != null) {
182+
writeID(writer.toString(), currentID);
183+
counter++;
184+
}
182185

183-
if ( currentID != null) {
184-
writeID(writer, currentID);
185-
counter++;
186+
currentID = line.substring(5);
187+
writer = new StringWriter();
186188
}
187189

188-
currentID = line.substring(5);
189-
writer = new StringWriter();
190+
writer.append(line);
191+
writer.append(NEWLINE);
192+
193+
line = buf.readLine ();
190194
}
191195

192-
writer.append(line);
193-
writer.append(NEWLINE);
196+
// write the last record...
197+
writeID(writer.toString(),currentID);
198+
counter++;
194199

195-
line = buf.readLine ();
196200
}
197201

198-
// write the last record...
199-
writeID(writer,currentID);
200-
counter++;
201-
202-
inStream.close();
203-
204202
logger.info("Created " + counter + " chemical component files.");
205203
}
206204

207-
private void writeID(StringWriter writer, String currentID) throws IOException{
205+
/**
206+
* Output chemical contents to a file
207+
* @param contents File contents
208+
* @param currentID Chemical ID, used to determine the filename
209+
* @throws IOException
210+
*/
211+
private void writeID(String contents, String currentID) throws IOException{
208212

209-
210213
String localName = DownloadChemCompProvider.getLocalFileName(currentID);
211214

212-
FileOutputStream outPut = new FileOutputStream(localName);
213-
214-
GZIPOutputStream gzOutPut = new GZIPOutputStream(outPut);
215-
216-
PrintWriter pw = new PrintWriter(gzOutPut);
217-
218-
pw.print(writer.toString());
219-
writer.close();
220-
pw.flush();
221-
pw.close();
222-
223-
outPut.close();
215+
try ( PrintWriter pw = new PrintWriter(new GZIPOutputStream(new FileOutputStream(localName))) ) {
224216

217+
pw.print(contents.toString());
218+
pw.flush();
219+
}
225220
}
226221

227222
/**
@@ -354,7 +349,7 @@ private static boolean downloadChemCompRecord(String recordName) {
354349

355350
String u = SERVER_LOCATION + recordName + ".cif";
356351

357-
// System.out.println("downloading " + u);
352+
logger.debug("downloading " + u);
358353

359354
URL url = null;
360355

@@ -364,29 +359,20 @@ private static boolean downloadChemCompRecord(String recordName) {
364359

365360
HttpURLConnection uconn = HTTPConnectionTools.openHttpURLConnection(url);
366361

367-
InputStream conn = uconn.getInputStream();
368-
369-
FileOutputStream outPut = new FileOutputStream(localName);
370-
371-
GZIPOutputStream gzOutPut = new GZIPOutputStream(outPut);
362+
try( PrintWriter pw = new PrintWriter(new GZIPOutputStream(new FileOutputStream(localName)));
363+
BufferedReader fileBuffer = new BufferedReader(new InputStreamReader(uconn.getInputStream()));
364+
) {
372365

373-
PrintWriter pw = new PrintWriter(gzOutPut);
366+
String line;
374367

375-
BufferedReader fileBuffer = new BufferedReader(new InputStreamReader(conn));
368+
while ((line = fileBuffer.readLine()) != null) {
369+
pw.println(line);
370+
}
376371

377-
String line;
372+
pw.flush();
378373

379-
while ((line = fileBuffer.readLine()) != null) {
380-
pw.println(line);
374+
return true;
381375
}
382-
383-
pw.flush();
384-
pw.close();
385-
386-
outPut.close();
387-
conn.close();
388-
389-
return true;
390376
} catch (FileNotFoundException e) {
391377
// Possible that there is no ChemComp matching this group.
392378
logger.warn(recordName + " is not available from " + SERVER_LOCATION + " and will be skipped");

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

Lines changed: 27 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,16 @@
2020
*/
2121
package org.biojava.nbio.structure.io.mmcif;
2222

23-
import org.biojava.nbio.structure.io.mmcif.chem.PolymerType;
24-
import org.biojava.nbio.structure.io.mmcif.chem.ResidueType;
25-
import org.biojava.nbio.structure.io.mmcif.model.ChemComp;
26-
import org.slf4j.Logger;
27-
import org.slf4j.LoggerFactory;
28-
2923
import java.io.BufferedReader;
3024
import java.io.IOException;
3125
import java.io.InputStream;
3226
import java.io.InputStreamReader;
3327
import java.util.zip.GZIPInputStream;
3428

29+
import org.biojava.nbio.structure.io.mmcif.model.ChemComp;
30+
import org.slf4j.Logger;
31+
import org.slf4j.LoggerFactory;
32+
3533

3634
/** Unlike the {@link DownloadChemCompProvider}, this {@link ChemCompProvider} does not download any chem comp definitions.
3735
* It has access to a limited set of files that are part of the biojava distribution.
@@ -46,54 +44,45 @@ public class ReducedChemCompProvider implements ChemCompProvider {
4644
public ReducedChemCompProvider(){
4745
logger.debug("Initialising ReducedChemCompProvider");
4846
}
49-
50-
51-
public ChemComp getEmptyChemComp(){
52-
ChemComp comp = new ChemComp();
53-
54-
comp.setOne_letter_code("?");
55-
comp.setPolymerType(PolymerType.unknown);
56-
comp.setResidueType(ResidueType.atomn);
57-
return comp;
58-
}
59-
47+
48+
6049
@Override
6150
public ChemComp getChemComp(String recordName) {
6251
String name = recordName.toUpperCase().trim();
63-
InputStream inStream = this.getClass().getResourceAsStream("/chemcomp/"+name + ".cif.gz");
64-
65-
if ( inStream == null){
66-
//System.out.println("Could not find chem comp: " + name + " ... using generic Chem Comp");
67-
// could not find the chem comp definition for this in the jar file
68-
logger.debug("Getting empty chem comp for {}",name);
69-
ChemComp cc = getEmptyChemComp();
70-
cc.setId(name);
71-
return cc;
72-
}
52+
try(InputStream inStream = this.getClass().getResourceAsStream("/chemcomp/"+name + ".cif.gz")) {
7353

74-
MMcifParser parser = new SimpleMMcifParser();
54+
logger.debug("Reading chemcomp/"+name+".cif.gz");
7555

76-
ChemCompConsumer consumer = new ChemCompConsumer();
56+
if ( inStream == null){
57+
//System.out.println("Could not find chem comp: " + name + " ... using generic Chem Comp");
58+
// could not find the chem comp definition for this in the jar file
59+
logger.debug("Getting empty chem comp for {}",name);
60+
ChemComp cc = ChemComp.getEmptyChemComp();
61+
cc.setId(name);
62+
return cc;
63+
}
7764

78-
// The Consumer builds up the BioJava - structure object.
79-
// you could also hook in your own and build up you own data model.
80-
parser.addMMcifConsumer(consumer);
65+
MMcifParser parser = new SimpleMMcifParser();
66+
67+
ChemCompConsumer consumer = new ChemCompConsumer();
68+
69+
// The Consumer builds up the BioJava - structure object.
70+
// you could also hook in your own and build up you own data model.
71+
parser.addMMcifConsumer(consumer);
8172

82-
try {
8373
parser.parse(new BufferedReader(new InputStreamReader(new GZIPInputStream(inStream))));
8474

8575
ChemicalComponentDictionary dict = consumer.getDictionary();
8676

8777
ChemComp chemComp = dict.getChemComp(name);
88-
78+
8979
return chemComp;
90-
} catch (IOException e){
91-
logger.error("IOException caught while reading chem comp {}. Error: {}",name,e.getMessage());
92-
//e.printStackTrace();
9380

81+
} catch (IOException e){
82+
logger.error("IOException caught while reading chem comp {}.",name,e);
9483
}
9584
logger.warn("Problem when loading chem comp {}, will use an empty chem comp for it", name);
96-
ChemComp cc = getEmptyChemComp();
85+
ChemComp cc = ChemComp.getEmptyChemComp();
9786
cc.setId(name);
9887
return cc;
9988
}

biojava-structure/src/main/java/org/biojava/nbio/structure/io/mmcif/model/ChemComp.java

Lines changed: 54 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -40,40 +40,40 @@ public class ChemComp implements Serializable, Comparable<ChemComp>{
4040
*/
4141
private static final long serialVersionUID = -4736341142030215915L;
4242

43-
String id ;
44-
String name;
45-
String type;
46-
String pdbx_type;
47-
String formula;
48-
String mon_nstd_parent_comp_id;
49-
String pdbx_synonyms;
50-
String pdbx_formal_charge;
51-
String pdbx_initial_date ;
52-
String pdbx_modified_date;
53-
String pdbx_ambiguous_flag;
54-
String pdbx_release_status ;
55-
String pdbx_replaced_by;
56-
String pdbx_replaces;
57-
String formula_weight;
58-
String one_letter_code;
59-
String three_letter_code;
60-
String pdbx_model_coordinates_details;
61-
String pdbx_model_coordinates_missing_flag;
62-
String pdbx_ideal_coordinates_details;
63-
String pdbx_ideal_coordinates_missing_flag;
64-
String pdbx_model_coordinates_db_code;
65-
String pdbx_subcomponent_list;
66-
String pdbx_processing_site;
67-
String mon_nstd_flag;
68-
69-
List<ChemCompDescriptor> descriptors = new ArrayList<ChemCompDescriptor>();
70-
List<ChemCompBond> bonds = new ArrayList<ChemCompBond>();
71-
List<ChemCompAtom> atoms = new ArrayList<ChemCompAtom>();
43+
private String id ;
44+
private String name;
45+
private String type;
46+
private String pdbx_type;
47+
private String formula;
48+
private String mon_nstd_parent_comp_id;
49+
private String pdbx_synonyms;
50+
private String pdbx_formal_charge;
51+
private String pdbx_initial_date ;
52+
private String pdbx_modified_date;
53+
private String pdbx_ambiguous_flag;
54+
private String pdbx_release_status ;
55+
private String pdbx_replaced_by;
56+
private String pdbx_replaces;
57+
private String formula_weight;
58+
private String one_letter_code;
59+
private String three_letter_code;
60+
private String pdbx_model_coordinates_details;
61+
private String pdbx_model_coordinates_missing_flag;
62+
private String pdbx_ideal_coordinates_details;
63+
private String pdbx_ideal_coordinates_missing_flag;
64+
private String pdbx_model_coordinates_db_code;
65+
private String pdbx_subcomponent_list;
66+
private String pdbx_processing_site;
67+
private String mon_nstd_flag;
68+
69+
private List<ChemCompDescriptor> descriptors = new ArrayList<ChemCompDescriptor>();
70+
private List<ChemCompBond> bonds = new ArrayList<ChemCompBond>();
71+
private List<ChemCompAtom> atoms = new ArrayList<ChemCompAtom>();
7272

7373
// and some derived data for easier processing...
74-
ResidueType residueType;
75-
PolymerType polymerType;
76-
boolean standard;
74+
private ResidueType residueType;
75+
private PolymerType polymerType;
76+
private boolean standard;
7777

7878
@Override
7979
public String toString(){
@@ -585,6 +585,28 @@ public boolean equals(Object obj) {
585585
return false;
586586
return true;
587587
}
588+
589+
/**
590+
* Creates a new instance of the dummy empty ChemComp.
591+
* @return
592+
*/
593+
public static ChemComp getEmptyChemComp(){
594+
ChemComp comp = new ChemComp();
595+
596+
comp.setOne_letter_code("?");
597+
comp.setThree_letter_code("???"); // Main signal for isEmpty()
598+
comp.setPolymerType(PolymerType.unknown);
599+
comp.setResidueType(ResidueType.atomn);
600+
return comp;
601+
}
588602

603+
/**
604+
* Indicates whether this compound was created with
605+
* @return
606+
*/
607+
public boolean isEmpty() {
608+
// Is this the best flag for it being empty?
609+
return id == null || getThree_letter_code() == null || getThree_letter_code().equals("???");
610+
}
589611

590612
}

0 commit comments

Comments
 (0)