Skip to content

Commit a0fb6f2

Browse files
minleejaeminleejae
authored
refactor: use one foreign key reference state (#2740)
Signed-off-by: minleejae <mmj9808@gmail.com>
1 parent 74de5c7 commit a0fb6f2

3 files changed

Lines changed: 227 additions & 83 deletions

File tree

‎src/main/java/net/sf/jsqlparser/statement/create/table/ForeignKeyIndex.java‎

Lines changed: 28 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -13,83 +13,67 @@
1313
import net.sf.jsqlparser.statement.ReferentialAction;
1414
import net.sf.jsqlparser.statement.ReferentialAction.Action;
1515
import net.sf.jsqlparser.statement.ReferentialAction.Type;
16-
import net.sf.jsqlparser.statement.select.PlainSelect;
17-
18-
import java.util.ArrayList;
1916
import java.util.Collection;
20-
import java.util.Collections;
21-
import java.util.LinkedHashSet;
2217
import java.util.List;
2318
import java.util.function.Consumer;
2419
import net.sf.jsqlparser.expression.Expression;
25-
import java.util.Optional;
26-
import java.util.Set;
2720

2821
public class ForeignKeyIndex extends NamedConstraint {
2922

30-
private Table table;
31-
private List<String> referencedColumnNames;
32-
private Set<ReferentialAction> referentialActions = new LinkedHashSet<>(2);
33-
private ForeignKeyReference reference;
23+
private ForeignKeyReference reference = new ForeignKeyReference();
3424

25+
/** Returns the mutable reference shared by the structured and legacy accessors. */
3526
public ForeignKeyReference getReference() {
36-
if (reference == null) {
37-
reference = new ForeignKeyReference();
38-
reference.setTable(table);
39-
reference.setReferencedColumnNames(referencedColumnNames);
40-
for (ReferentialAction action : referentialActions) {
41-
reference.setReferentialAction(action.getType(), action.getAction());
42-
}
43-
}
4427
return reference;
4528
}
4629

30+
/**
31+
* Replaces the reference. A null value detaches its current table, referenced columns and
32+
* referential actions from reference-only options such as MATCH, preserving the legacy setter
33+
* contract without restoring stale values from before structured edits.
34+
*/
4735
public void setReference(ForeignKeyReference reference) {
48-
this.reference = reference;
49-
if (reference != null) {
50-
table = reference.getTable();
51-
referencedColumnNames = reference.getReferencedColumnNames();
52-
referentialActions.clear();
53-
referentialActions.addAll(reference.getReferentialActions());
36+
if (reference == null) {
37+
ForeignKeyReference detached = new ForeignKeyReference()
38+
.withTable(this.reference.getTable())
39+
.withReferencedColumnNames(this.reference.getReferencedColumnNames());
40+
detached.getReferentialActions().addAll(this.reference.getReferentialActions());
41+
this.reference = detached;
42+
} else {
43+
this.reference = reference;
5444
}
5545
}
5646

5747
public ForeignKeyReference.MatchType getMatchType() {
58-
return reference != null ? reference.getMatchType() : null;
48+
return reference.getMatchType();
5949
}
6050

6151
public void setMatchType(ForeignKeyReference.MatchType matchType) {
6252
getReference().setMatchType(matchType);
6353
}
6454

6555
public Table getTable() {
66-
return reference != null ? reference.getTable() : table;
56+
return reference.getTable();
6757
}
6858

6959
public void setTable(Table table) {
70-
this.table = table;
71-
if (reference != null) {
72-
reference.setTable(table);
73-
}
60+
reference.setTable(table);
7461
}
7562

7663
public List<String> getReferencedColumnNames() {
77-
return reference != null ? reference.getReferencedColumnNames() : referencedColumnNames;
64+
return reference.getReferencedColumnNames();
7865
}
7966

8067
public void setReferencedColumnNames(List<String> referencedColumnNames) {
81-
this.referencedColumnNames = referencedColumnNames;
82-
if (reference != null) {
83-
reference.setReferencedColumnNames(referencedColumnNames);
84-
}
68+
reference.setReferencedColumnNames(referencedColumnNames);
8569
}
8670

8771
/**
8872
* @param type
8973
* @param action
9074
*/
9175
public void setReferentialAction(Type type, Action action) {
92-
setReferentialAction(type, action, true);
76+
reference.setReferentialAction(type, action);
9377
}
9478

9579
public ForeignKeyIndex withReferentialAction(Type type, Action action) {
@@ -101,39 +85,15 @@ public ForeignKeyIndex withReferentialAction(Type type, Action action) {
10185
* @param type
10286
*/
10387
public void removeReferentialAction(Type type) {
104-
setReferentialAction(type, null, false);
88+
reference.removeReferentialAction(type);
10589
}
10690

10791
/**
10892
* @param type
10993
* @return
11094
*/
11195
public ReferentialAction getReferentialAction(Type type) {
112-
if (reference != null) {
113-
return reference.getReferentialAction(type);
114-
}
115-
return referentialActions.stream().filter(ra -> type.equals(ra.getType())).findFirst()
116-
.orElse(null);
117-
}
118-
119-
private void setReferentialAction(Type type, Action action, boolean set) {
120-
ReferentialAction found = getReferentialAction(type);
121-
if (set) {
122-
if (reference != null) {
123-
reference.setReferentialAction(type, action);
124-
return;
125-
}
126-
if (found == null) {
127-
referentialActions.add(new ReferentialAction(type, action));
128-
} else {
129-
found.setAction(action);
130-
}
131-
} else if (found != null) {
132-
referentialActions.remove(found);
133-
if (reference != null) {
134-
reference.removeReferentialAction(type);
135-
}
136-
}
96+
return reference.getReferentialAction(type);
13797
}
13898

13999
@Deprecated
@@ -169,14 +129,7 @@ public void setOnUpdateReferenceOption(String onUpdateReferenceOption) {
169129
@Override
170130
public void appendTo(StringBuilder b, Consumer<Expression> expressionPrinter) {
171131
super.appendTo(b, expressionPrinter);
172-
b.append(' ');
173-
if (reference != null) {
174-
b.append(reference);
175-
} else {
176-
b.append("REFERENCES ").append(table)
177-
.append(PlainSelect.getStringList(getReferencedColumnNames(), true, true));
178-
referentialActions.forEach(b::append);
179-
}
132+
b.append(' ').append(reference);
180133
appendConstraintSuffixTo(b);
181134
appendConstraintAttributesTo(b);
182135
}
@@ -212,17 +165,13 @@ public ForeignKeyIndex withOnUpdateReferenceOption(String onUpdateReferenceOptio
212165
}
213166

214167
public ForeignKeyIndex addReferencedColumnNames(String... referencedColumnNames) {
215-
List<String> collection =
216-
Optional.ofNullable(getReferencedColumnNames()).orElseGet(ArrayList::new);
217-
Collections.addAll(collection, referencedColumnNames);
218-
return this.withReferencedColumnNames(collection);
168+
reference.addReferencedColumnNames(referencedColumnNames);
169+
return this;
219170
}
220171

221172
public ForeignKeyIndex addReferencedColumnNames(Collection<String> referencedColumnNames) {
222-
List<String> collection =
223-
Optional.ofNullable(getReferencedColumnNames()).orElseGet(ArrayList::new);
224-
collection.addAll(referencedColumnNames);
225-
return this.withReferencedColumnNames(collection);
173+
reference.addReferencedColumnNames(referencedColumnNames);
174+
return this;
226175
}
227176

228177
@Override

‎src/main/java/net/sf/jsqlparser/statement/create/table/ForeignKeyReference.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,8 @@ public void setReferentialAction(Type type, Action action) {
116116
}
117117

118118
public void removeReferentialAction(Type type) {
119-
ReferentialAction current = getReferentialAction(type);
120-
if (current != null) {
121-
referentialActions.remove(current);
122-
}
119+
// Actions are mutable; their hash code can change after insertion into the set.
120+
referentialActions.removeIf(action -> type.equals(action.getType()));
123121
}
124122

125123
public ForeignKeyReference withTable(Table table) {

‎src/test/java/net/sf/jsqlparser/statement/create/ForeignKeyReferenceMutationTest.java‎

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
/*-
2+
* #%L
3+
* JSQLParser library
4+
* %%
5+
* Copyright (C) 2004 - 2026 JSQLParser
6+
* %%
7+
* Dual licensed under GNU LGPL 2.1 or Apache License 2.0
8+
* #L%
9+
*/
10+
package net.sf.jsqlparser.statement.create;
11+
12+
import static org.junit.jupiter.api.Assertions.assertEquals;
13+
import static org.junit.jupiter.api.Assertions.assertFalse;
14+
import static org.junit.jupiter.api.Assertions.assertNotSame;
15+
import static org.junit.jupiter.api.Assertions.assertNull;
16+
import static org.junit.jupiter.api.Assertions.assertSame;
17+
18+
import java.util.ArrayList;
19+
import java.util.List;
20+
import net.sf.jsqlparser.JSQLParserException;
21+
import net.sf.jsqlparser.parser.CCJSqlParserUtil;
22+
import net.sf.jsqlparser.schema.Table;
23+
import net.sf.jsqlparser.statement.ReferentialAction;
24+
import net.sf.jsqlparser.statement.ReferentialAction.Action;
25+
import net.sf.jsqlparser.statement.ReferentialAction.Type;
26+
import net.sf.jsqlparser.statement.Statement;
27+
import net.sf.jsqlparser.statement.alter.Alter;
28+
import net.sf.jsqlparser.statement.create.table.ConstraintAttributes;
29+
import net.sf.jsqlparser.statement.create.table.CreateTable;
30+
import net.sf.jsqlparser.statement.create.table.ForeignKeyIndex;
31+
import net.sf.jsqlparser.statement.create.table.ForeignKeyReference;
32+
import net.sf.jsqlparser.util.deparser.StatementDeParser;
33+
import org.junit.jupiter.api.Test;
34+
import org.junit.jupiter.params.ParameterizedTest;
35+
import org.junit.jupiter.params.provider.ValueSource;
36+
37+
class ForeignKeyReferenceMutationTest {
38+
@Test
39+
void keepsLegacyAndStructuredAccessOnTheSameValues() {
40+
ForeignKeyIndex index = new ForeignKeyIndex();
41+
Table table = new Table("parent");
42+
List<String> columns = new ArrayList<>(List.of("id"));
43+
index.setTable(table);
44+
index.setReferencedColumnNames(columns);
45+
index.setReferentialAction(Type.DELETE, Action.CASCADE);
46+
47+
ForeignKeyReference reference = index.getReference();
48+
assertSame(table, reference.getTable());
49+
assertSame(columns, reference.getReferencedColumnNames());
50+
assertEquals(Action.CASCADE, reference.getReferentialAction(Type.DELETE).getAction());
51+
52+
Table replacement = new Table("other_parent");
53+
reference.setTable(replacement);
54+
reference.setReferencedColumnNames(new ArrayList<>(List.of("other_id")));
55+
reference.setReferentialAction(Type.DELETE, Action.RESTRICT);
56+
assertSame(replacement, index.getTable());
57+
assertSame(reference.getReferencedColumnNames(), index.getReferencedColumnNames());
58+
assertEquals(Action.RESTRICT, index.getReferentialAction(Type.DELETE).getAction());
59+
60+
index.addReferencedColumnNames("tenant_id");
61+
index.getReferencedColumnNames().remove("other_id");
62+
assertEquals(List.of("tenant_id"), reference.getReferencedColumnNames());
63+
index.removeReferentialAction(Type.DELETE);
64+
assertNull(reference.getReferentialAction(Type.DELETE));
65+
reference.getReferentialActions().add(new ReferentialAction(Type.UPDATE, Action.CASCADE));
66+
assertSame(reference.getReferentialAction(Type.UPDATE),
67+
index.getReferentialAction(Type.UPDATE));
68+
}
69+
70+
@Test
71+
void preservesNullableLegacyFieldsAndAnEmptyReference() {
72+
ForeignKeyIndex index = new ForeignKeyIndex();
73+
index.setReference(null);
74+
assertNull(index.getTable());
75+
assertNull(index.getReferencedColumnNames());
76+
assertNull(index.getReferentialAction(Type.DELETE));
77+
assertNull(index.getMatchType());
78+
index.setTable(new Table("parent"));
79+
index.setReferencedColumnNames(new ArrayList<>(List.of("id")));
80+
index.setTable(null);
81+
index.setReferencedColumnNames(null);
82+
assertNull(index.getReference().getTable());
83+
assertNull(index.getReference().getReferencedColumnNames());
84+
}
85+
86+
@Test
87+
void clearingReferenceRetainsLegacyValuesAndDiscardsReferenceOnlyOptions() {
88+
ForeignKeyReference reference = new ForeignKeyReference()
89+
.withTable(new Table("parent"))
90+
.withReferencedColumnNames(new ArrayList<>(List.of("id")))
91+
.withMatchType(ForeignKeyReference.MatchType.FULL)
92+
.withUsingPeriod(true)
93+
.withReferentialAction(Type.DELETE, Action.CASCADE);
94+
reference.setConstraintName("column_fk");
95+
reference.setConstraintAttributes(new ConstraintAttributes());
96+
ForeignKeyIndex index = new ForeignKeyIndex().withReference(reference);
97+
98+
index.setReference(null);
99+
100+
assertSame(reference.getTable(), index.getTable());
101+
assertSame(reference.getReferencedColumnNames(), index.getReferencedColumnNames());
102+
assertEquals(Action.CASCADE, index.getReferentialAction(Type.DELETE).getAction());
103+
assertNull(index.getMatchType());
104+
ForeignKeyReference detached = index.getReference();
105+
assertNotSame(reference, detached);
106+
assertFalse(detached.isUsingPeriod());
107+
assertNull(detached.getConstraintName());
108+
assertNull(detached.getConstraintAttributes());
109+
}
110+
111+
@Test
112+
void clearingReferenceRetainsTheLatestStructuredValues() {
113+
ForeignKeyReference reference = new ForeignKeyReference()
114+
.withTable(new Table("parent"))
115+
.withReferencedColumnNames(new ArrayList<>(List.of("id")));
116+
ForeignKeyIndex index = new ForeignKeyIndex().withReference(reference);
117+
Table replacement = new Table("other_parent");
118+
List<String> columns = new ArrayList<>(List.of("other_id"));
119+
reference.setTable(replacement);
120+
reference.setReferencedColumnNames(columns);
121+
reference.setReferentialAction(Type.DELETE, Action.SET_NULL);
122+
reference.getReferentialAction(Type.DELETE).setColumnNames(List.of("child_id"));
123+
124+
index.setReference(null);
125+
126+
assertSame(replacement, index.getTable());
127+
assertSame(columns, index.getReferencedColumnNames());
128+
assertEquals(List.of("child_id"), index.getReferentialAction(Type.DELETE).getColumnNames());
129+
assertEquals(List.of("child_id"),
130+
index.getReference().getReferentialAction(Type.DELETE).getColumnNames());
131+
}
132+
133+
@Test
134+
void accessingReferenceDoesNotReplaceActionsOrDiscardTheirColumns() {
135+
ForeignKeyIndex index = new ForeignKeyIndex();
136+
index.setReferentialAction(Type.DELETE, Action.SET_NULL);
137+
ReferentialAction action = index.getReferentialAction(Type.DELETE);
138+
action.setColumnNames(List.of("child_id"));
139+
140+
assertSame(action, index.getReference().getReferentialAction(Type.DELETE));
141+
assertEquals(List.of("child_id"), index.getReferentialAction(Type.DELETE).getColumnNames());
142+
action.setAction(Action.SET_DEFAULT);
143+
assertEquals(Action.SET_DEFAULT,
144+
index.getReference().getReferentialAction(Type.DELETE).getAction());
145+
}
146+
147+
@Test
148+
@SuppressWarnings("deprecation")
149+
void deprecatedActionSettersUpdateStructuredReference() {
150+
ForeignKeyIndex index = new ForeignKeyIndex();
151+
ForeignKeyReference reference = index.getReference();
152+
index.setOnDeleteReferenceOption("CASCADE");
153+
index.setOnUpdateReferenceOption("RESTRICT");
154+
assertEquals(Action.CASCADE, reference.getReferentialAction(Type.DELETE).getAction());
155+
assertEquals("RESTRICT", index.getOnUpdateReferenceOption());
156+
reference.setReferentialAction(Type.UPDATE, Action.CASCADE);
157+
assertEquals("CASCADE", index.getOnUpdateReferenceOption());
158+
index.setOnDeleteReferenceOption(null);
159+
index.setOnUpdateReferenceOption(null);
160+
assertNull(reference.getReferentialAction(Type.DELETE));
161+
assertNull(reference.getReferentialAction(Type.UPDATE));
162+
}
163+
164+
@ParameterizedTest
165+
@ValueSource(booleans = {false, true})
166+
void mutationsSurviveBothRenderersAndReparsing(boolean create) throws JSQLParserException {
167+
String prefix = create ? "CREATE TABLE child (parent_id INT, " : "ALTER TABLE child ADD ";
168+
Statement statement = CCJSqlParserUtil.parse(prefix
169+
+ "CONSTRAINT fk FOREIGN KEY (parent_id) REFERENCES parent (id) ON DELETE CASCADE"
170+
+ (create ? ")" : ""));
171+
ForeignKeyIndex index = foreignKey(statement);
172+
ForeignKeyReference reference = index.getReference();
173+
reference.setTable(new Table("other_parent"));
174+
reference.getReferencedColumnNames().set(0, "other_id");
175+
index.setReferentialAction(Type.DELETE, Action.SET_NULL);
176+
index.getReferentialAction(Type.DELETE).setColumnNames(List.of("parent_id"));
177+
index.setMatchType(ForeignKeyReference.MatchType.SIMPLE);
178+
179+
StringBuilder deparsed = new StringBuilder();
180+
statement.accept(new StatementDeParser(deparsed));
181+
assertEquals(statement.toString(), deparsed.toString());
182+
Statement reparsed = CCJSqlParserUtil.parse(deparsed.toString());
183+
assertEquals(deparsed.toString(), reparsed.toString());
184+
ForeignKeyIndex result = foreignKey(reparsed);
185+
assertEquals("other_parent", result.getTable().getName());
186+
assertEquals(List.of("other_id"), result.getReferencedColumnNames());
187+
assertEquals(List.of("parent_id"),
188+
result.getReferentialAction(Type.DELETE).getColumnNames());
189+
assertEquals(ForeignKeyReference.MatchType.SIMPLE, result.getMatchType());
190+
}
191+
192+
private static ForeignKeyIndex foreignKey(Statement statement) {
193+
return (ForeignKeyIndex) (statement instanceof CreateTable
194+
? ((CreateTable) statement).getIndexes().get(0)
195+
: ((Alter) statement).getAlterExpressions().get(0).getIndex());
196+
}
197+
}

0 commit comments

Comments
 (0)