diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/cif/AbstractCifFileSupplier.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/cif/AbstractCifFileSupplier.java index 6bf8af90ef..e43565c827 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/cif/AbstractCifFileSupplier.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/cif/AbstractCifFileSupplier.java @@ -40,8 +40,27 @@ protected CifFile getInternal(Structure structure, List wrappedAtom // entity information List entityInfos = structure.getEntityInfos(); + PdbId pdbId = structure.getPdbId(); + MmCifBlockBuilder blockBuilder = CifBuilder.enterFile(StandardSchemata.MMCIF) - .enterBlock(structure.getPdbId() == null? "" : structure.getPdbId().getId()); + .enterBlock(pdbId == null? "" : pdbId.getId()); + + if (pdbId != null) { + // The block header alone does not carry the identifier for consumers: readers pick it up from + // _entry.id (e.g. Jmol) or from _struct.entry_id (BioJava's own CifStructureConsumerImpl). + // Both are written so that the identifier survives a write-then-read round trip either way. + blockBuilder.enterEntry() + .enterId() + .add(pdbId.getId()) + .leaveColumn() + .leaveCategory(); + + blockBuilder.enterStruct() + .enterEntryId() + .add(pdbId.getId()) + .leaveColumn() + .leaveCategory(); + } blockBuilder.enterStructKeywords().enterText() .add(String.join(", ", structure.getPDBHeader().getKeywords())) diff --git a/biojava-structure/src/test/java/org/biojava/nbio/structure/io/cif/CifFileSupplierImplTest.java b/biojava-structure/src/test/java/org/biojava/nbio/structure/io/cif/CifFileSupplierImplTest.java index df227a8669..1d5f496ff5 100644 --- a/biojava-structure/src/test/java/org/biojava/nbio/structure/io/cif/CifFileSupplierImplTest.java +++ b/biojava-structure/src/test/java/org/biojava/nbio/structure/io/cif/CifFileSupplierImplTest.java @@ -1,5 +1,6 @@ package org.biojava.nbio.structure.io.cif; +import org.biojava.nbio.structure.PdbId; import org.biojava.nbio.structure.Structure; import org.biojava.nbio.structure.io.FileParsingParameters; import org.biojava.nbio.structure.io.PDBFileParser; @@ -41,4 +42,43 @@ public void shouldReadRawPdbOutputtingCifWithEntity() throws IOException { } } + + /** + * The identifier must be written as a data item and not only as the name of the data block: consumers read it + * from _entry.id or from _struct.entry_id, so writing the block header alone loses it. See issue #1143. + */ + @Test + public void shouldWriteEntryIdAndSurviveRoundTrip() throws IOException { + Structure s; + try (InputStream inStream = new GZIPInputStream(this.getClass().getResourceAsStream("/4hhb.cif.gz"))) { + s = CifStructureConverter.fromInputStream(inStream); + } + assertEquals(new PdbId("4HHB"), s.getPdbId()); + + String cifText = CifStructureConverter.toText(s); + assertTrue("_entry.id must be written", cifText.contains("_entry.id")); + assertTrue("_struct.entry_id must be written", cifText.contains("_struct.entry_id")); + + Structure readStruct = CifStructureConverter.fromInputStream( + new ByteArrayInputStream(cifText.getBytes())); + + assertEquals(s.getPdbId(), readStruct.getPdbId()); + assertEquals(s.getPdbId(), readStruct.getPDBHeader().getPdbId()); + } + + /** + * Structures without an identifier must not gain empty entry categories. + */ + @Test + public void shouldNotWriteEntryIdWhenPdbIdIsAbsent() throws IOException { + Structure s; + try (InputStream inStream = new GZIPInputStream(this.getClass().getResourceAsStream("/4hhb.cif.gz"))) { + s = CifStructureConverter.fromInputStream(inStream); + } + s.setPdbId(null); + + String cifText = CifStructureConverter.toText(s); + assertFalse(cifText.contains("_entry.id")); + assertFalse(cifText.contains("_struct.entry_id")); + } }