Skip to content

Commit be306cd

Browse files
jcschaffclaude
jcschaff
and
claude
committed
Fix publication API crash when BioModel names contain commas
The SQL listagg/string_agg used comma as the record separator for aggregated model references, which broke parsing when model names contained commas. Changed separator to '@@' and extracted parsing into testable static methods. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c709a12 commit be306cd

2 files changed

Lines changed: 112 additions & 18 deletions

File tree

‎vcell-server/src/main/java/cbit/vcell/modeldb/PublicationTable.java‎

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public String getPreparedStatement_PublicationReps(String conditions, OrderBy or
8888
PublicationModelLinkTable pubModelTable = PublicationModelLinkTable.table;
8989

9090
String concat_function_name = (dbSyntax==DatabaseSyntax.ORACLE) ? "listagg" : "string_agg";
91-
String concat_second_arg = (dbSyntax==DatabaseSyntax.ORACLE) ? ", ','" : ", ','";
91+
String concat_second_arg = (dbSyntax==DatabaseSyntax.ORACLE) ? ", '@@'" : ", '@@'";
9292
String string_cast = (dbSyntax==DatabaseSyntax.ORACLE) ? "" : "::varchar(255)";
9393

9494
String subquery =
@@ -178,44 +178,54 @@ public PublicationRep getPublicationRep(ResultSet rset, DatabaseSyntax dbSyntax)
178178
java.util.Date pubdate = VersionTable.getDate(rset,dbSyntax,table.pubdate.toString());
179179

180180
String bmRefsString = rset.getString("bmRefs");
181+
BioModelReferenceRep[] bmRefArray = parseBioModelRefs(bmRefsString);
182+
183+
String mmRefsString = rset.getString("mmRefs");
184+
MathModelReferenceRep[] mmRefArray = parseMathModelRefs(mmRefsString);
185+
186+
187+
return new PublicationRep(pubKey,title,authorsList.split(";"),year,citation,pubmedid,doi,endnoteid,url,bmRefArray,mmRefArray,wittid,pubdate);
188+
}
189+
190+
static final String RECORD_SEPARATOR = "@@";
191+
192+
static BioModelReferenceRep[] parseBioModelRefs(String bmRefsString) {
181193
ArrayList<BioModelReferenceRep> bmRefList = new ArrayList<BioModelReferenceRep>();
182-
if(bmRefsString != null && bmRefsString.length()>0) {
183-
String[] bmRefStrings = bmRefsString.replace("[", "").replace("]", "").split(",");
194+
if (bmRefsString != null && bmRefsString.length() > 0) {
195+
String[] bmRefStrings = bmRefsString.replace("[", "").replace("]", "").split(RECORD_SEPARATOR);
184196
for (String bmRefString : bmRefStrings) {
185-
String bmRefComponents[] = bmRefString.split(";");
186-
if (bmRefComponents.length >= 5){
197+
String[] bmRefComponents = bmRefString.split(";");
198+
if (bmRefComponents.length >= 5) {
187199
KeyValue bmKey = new KeyValue(bmRefComponents[0]);
188200
String bmName = bmRefComponents[1];
189201
KeyValue ownerKey = new KeyValue(bmRefComponents[2]);
190202
String ownerUserid = bmRefComponents[3];
191203
Long versionFlag = Long.valueOf(bmRefComponents[4]);
192204
Integer privacy = bmRefComponents.length >= 6 ? Integer.valueOf(bmRefComponents[5]) : null;
193-
bmRefList.add(new BioModelReferenceRep(bmKey, bmName, new User(ownerUserid,ownerKey), versionFlag, privacy));
205+
bmRefList.add(new BioModelReferenceRep(bmKey, bmName, new User(ownerUserid, ownerKey), versionFlag, privacy));
194206
}
195207
}
196208
}
197-
BioModelReferenceRep[] bmRefArray = bmRefList.toArray(new BioModelReferenceRep[0]);
198-
199-
String mmRefsString = rset.getString("mmRefs");
209+
return bmRefList.toArray(new BioModelReferenceRep[0]);
210+
}
211+
212+
static MathModelReferenceRep[] parseMathModelRefs(String mmRefsString) {
200213
ArrayList<MathModelReferenceRep> mmRefList = new ArrayList<MathModelReferenceRep>();
201-
if(mmRefsString != null && mmRefsString.length() > 0) {
202-
String[] mmRefStrings = mmRefsString.replace("[", "").replace("]", "").split(",");
214+
if (mmRefsString != null && mmRefsString.length() > 0) {
215+
String[] mmRefStrings = mmRefsString.replace("[", "").replace("]", "").split(RECORD_SEPARATOR);
203216
for (String mmRefString : mmRefStrings) {
204-
String mmRefComponents[] = mmRefString.split(";");
205-
if (mmRefComponents.length==5){
217+
String[] mmRefComponents = mmRefString.split(";");
218+
if (mmRefComponents.length == 5) {
206219
KeyValue mmKey = new KeyValue(mmRefComponents[0]);
207220
String mmName = mmRefComponents[1];
208221
KeyValue ownerKey = new KeyValue(mmRefComponents[2]);
209222
String ownerUserid = mmRefComponents[3];
210223
Long versionFlag = Long.valueOf(mmRefComponents[4]);
211-
mmRefList.add(new MathModelReferenceRep(mmKey, mmName, new User(ownerUserid,ownerKey),versionFlag));
224+
mmRefList.add(new MathModelReferenceRep(mmKey, mmName, new User(ownerUserid, ownerKey), versionFlag));
212225
}
213226
}
214227
}
215-
MathModelReferenceRep[] mmRefArray = mmRefList.toArray(new MathModelReferenceRep[0]);
216-
217-
218-
return new PublicationRep(pubKey,title,authorsList.split(";"),year,citation,pubmedid,doi,endnoteid,url,bmRefArray,mmRefArray,wittid,pubdate);
228+
return mmRefList.toArray(new MathModelReferenceRep[0]);
219229
}
220230

221231
}

‎vcell-server/src/test/java/cbit/vcell/modeldb/PublicationTableTest.java‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
package cbit.vcell.modeldb;
2+
3+
import org.junit.jupiter.api.Tag;
4+
import org.junit.jupiter.api.Test;
5+
6+
import static org.junit.jupiter.api.Assertions.*;
7+
8+
@Tag("Fast")
9+
public class PublicationTableTest {
10+
11+
@Test
12+
public void testParseBioModelRefs_simple() {
13+
String bmRefs = "[100;MyModel;200;jsmith;0;1]";
14+
BioModelReferenceRep[] refs = PublicationTable.parseBioModelRefs(bmRefs);
15+
assertEquals(1, refs.length);
16+
assertEquals("100", refs[0].getBmKey().toString());
17+
assertEquals("MyModel", refs[0].getName());
18+
assertEquals("200", refs[0].getOwner().getID().toString());
19+
assertEquals("jsmith", refs[0].getOwner().getName());
20+
assertEquals(0L, refs[0].getVersionFlag());
21+
assertEquals(1, refs[0].getPrivacy());
22+
}
23+
24+
@Test
25+
public void testParseBioModelRefs_multiple() {
26+
String bmRefs = "[100;Model A;200;alice;0;1@@300;Model B;400;bob;2;0]";
27+
BioModelReferenceRep[] refs = PublicationTable.parseBioModelRefs(bmRefs);
28+
assertEquals(2, refs.length);
29+
assertEquals("Model A", refs[0].getName());
30+
assertEquals("Model B", refs[1].getName());
31+
}
32+
33+
@Test
34+
public void testParseBioModelRefs_nameWithComma() {
35+
// This was the production bug: model names containing commas broke the old comma-based split
36+
String bmRefs = "[100;Smith, Jones 2024;200;jsmith;0;1@@300;Other Model;400;bob;2;0]";
37+
BioModelReferenceRep[] refs = PublicationTable.parseBioModelRefs(bmRefs);
38+
assertEquals(2, refs.length);
39+
assertEquals("Smith, Jones 2024", refs[0].getName());
40+
assertEquals("100", refs[0].getBmKey().toString());
41+
assertEquals("Other Model", refs[1].getName());
42+
assertEquals("300", refs[1].getBmKey().toString());
43+
}
44+
45+
@Test
46+
public void testParseBioModelRefs_withoutPrivacy() {
47+
String bmRefs = "[100;MyModel;200;jsmith;0]";
48+
BioModelReferenceRep[] refs = PublicationTable.parseBioModelRefs(bmRefs);
49+
assertEquals(1, refs.length);
50+
assertNull(refs[0].getPrivacy());
51+
}
52+
53+
@Test
54+
public void testParseBioModelRefs_nullAndEmpty() {
55+
assertEquals(0, PublicationTable.parseBioModelRefs(null).length);
56+
assertEquals(0, PublicationTable.parseBioModelRefs("").length);
57+
}
58+
59+
@Test
60+
public void testParseMathModelRefs_simple() {
61+
String mmRefs = "[100;MyMathModel;200;jsmith;0]";
62+
MathModelReferenceRep[] refs = PublicationTable.parseMathModelRefs(mmRefs);
63+
assertEquals(1, refs.length);
64+
assertEquals("100", refs[0].getMmKey().toString());
65+
assertEquals("MyMathModel", refs[0].getName());
66+
assertEquals("jsmith", refs[0].getOwner().getName());
67+
assertEquals(0L, refs[0].getVersionFlag());
68+
}
69+
70+
@Test
71+
public void testParseMathModelRefs_nameWithComma() {
72+
String mmRefs = "[100;Model, revised;200;jsmith;0@@300;Other;400;bob;2]";
73+
MathModelReferenceRep[] refs = PublicationTable.parseMathModelRefs(mmRefs);
74+
assertEquals(2, refs.length);
75+
assertEquals("Model, revised", refs[0].getName());
76+
assertEquals("Other", refs[1].getName());
77+
}
78+
79+
@Test
80+
public void testParseMathModelRefs_nullAndEmpty() {
81+
assertEquals(0, PublicationTable.parseMathModelRefs(null).length);
82+
assertEquals(0, PublicationTable.parseMathModelRefs("").length);
83+
}
84+
}

0 commit comments

Comments
 (0)