Skip to content

Commit dfb306a

Browse files
punktiliouspunktilious
authored
Merge pull request #2971 from IBM/issue-2050-fix1
Issue 2050 fix 1 correct locationURI when IfNoneMatch
2 parents 7682071 + efacf2a commit dfb306a

13 files changed

Lines changed: 196 additions & 37 deletions

File tree

‎fhir-model/src/main/java/com/ibm/fhir/model/util/FHIRUtil.java‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,11 +262,39 @@ public static OperationOutcome buildOperationOutcome(String message, IssueType i
262262
* <code>"[resource-type]/[id]/_history/[version]"</code>. Note that the server will turn this into an absolute URL prior to
263263
* returning it to the client.
264264
*
265+
* @param type
266+
* the resource type name
265267
* @param resource
266268
* the resource for which the location header value should be returned
267269
*/
268270
public static URI buildLocationURI(String type, Resource resource) {
269-
return URI.create(type + "/" + resource.getId() + "/_history/" + resource.getMeta().getVersionId().getValue());
271+
StringBuilder sb = new StringBuilder();
272+
sb.append(type);
273+
sb.append("/");
274+
sb.append(resource.getId());
275+
sb.append("/_history/");
276+
sb.append(resource.getMeta().getVersionId().getValue());
277+
return URI.create(sb.toString());
278+
}
279+
280+
/**
281+
* Builds a relative "Location" header value for the specified resource type/id/version. This will be a string of the form
282+
* <code>"[resource-type]/[id]/_history/[version]"</code>. Note that the server will turn this into an absolute URL prior to
283+
* returning it to the client.
284+
*
285+
* @param type the resource type name
286+
* @param id the resource logical id value
287+
* @param version the resource version
288+
* @return
289+
*/
290+
public static URI buildLocationURI(String type, String id, int version) {
291+
StringBuilder sb = new StringBuilder();
292+
sb.append(type);
293+
sb.append("/");
294+
sb.append(id);
295+
sb.append("/_history/");
296+
sb.append(version);
297+
return URI.create(sb.toString());
270298
}
271299

272300
/**

‎fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dao/impl/ResourceDAOImpl.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,10 @@ public class ResourceDAOImpl extends FHIRDbDAOImpl implements ResourceDAO {
9191

9292
// @formatter:off
9393
// 0 1
94-
// 1 2 3 4 5 6 7 8 9 0 1 2
94+
// 1 2 3 4 5 6 7 8 9 0 1 2 3
9595
// @formatter:on
9696
// Don't forget that we must account for IN and OUT parameters.
97-
private static final String SQL_INSERT_WITH_PARAMETERS = "CALL %s.add_any_resource(?,?,?,?,?,?,?,?,?,?,?,?)";
97+
private static final String SQL_INSERT_WITH_PARAMETERS = "CALL %s.add_any_resource(?,?,?,?,?,?,?,?,?,?,?,?,?)";
9898

9999
// Read version history of the resource identified by its logical-id
100100
private static final String SQL_HISTORY =
@@ -544,6 +544,7 @@ public Resource insert(Resource resource, List<ExtractedParameterValue> paramete
544544
stmt.registerOutParameter(10, Types.BIGINT); // resource_id
545545
stmt.registerOutParameter(11, Types.VARCHAR); // current_hash
546546
stmt.registerOutParameter(12, Types.INTEGER); // o_interaction_status
547+
stmt.registerOutParameter(13, Types.INTEGER); // o_if_none_match_version
547548

548549
stmt.execute();
549550
long latestTime = System.nanoTime();
@@ -556,6 +557,7 @@ public Resource insert(Resource resource, List<ExtractedParameterValue> paramete
556557
if (interactionStatus == 1) {
557558
// No update, so no need to make any more changes
558559
resource.setInteractionStatus(InteractionStatus.IF_NONE_MATCH_EXISTED);
560+
resource.setIfNoneMatchVersion(stmt.getInt(13));
559561
} else {
560562
resource.setInteractionStatus(InteractionStatus.MODIFIED);
561563

‎fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/derby/DerbyResourceDAO.java‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import java.util.Calendar;
1717
import java.util.List;
1818
import java.util.UUID;
19+
import java.util.concurrent.atomic.AtomicInteger;
1920
import java.util.logging.Level;
2021
import java.util.logging.Logger;
2122

@@ -113,6 +114,10 @@ public Resource insert(Resource resource, List<ExtractedParameterValue> paramet
113114

114115
final String sourceKey = UUID.randomUUID().toString();
115116

117+
// to mimic out parameters from the stored procedure
118+
AtomicInteger outInteractionStatus = new AtomicInteger();
119+
AtomicInteger outIfNoneMatchVersion = new AtomicInteger();
120+
116121
long resourceId = this.storeResource(resource.getResourceType(),
117122
parameters,
118123
resource.getLogicalId(),
@@ -124,14 +129,21 @@ public Resource insert(Resource resource, List<ExtractedParameterValue> paramet
124129
parameterHashB64,
125130
connection,
126131
parameterDao,
127-
ifNoneMatch
132+
ifNoneMatch,
133+
outInteractionStatus,
134+
outIfNoneMatchVersion
128135
);
129136

130137

131138
dbCallDuration = (System.nanoTime() - dbCallStartTime)/1e6;
132139

133-
resource.setId(resourceId);
134-
resource.setInteractionStatus(InteractionStatus.MODIFIED);
140+
if (outInteractionStatus.get() == 1) {
141+
resource.setInteractionStatus(InteractionStatus.IF_NONE_MATCH_EXISTED);
142+
resource.setIfNoneMatchVersion(outIfNoneMatchVersion.get());
143+
} else {
144+
resource.setInteractionStatus(InteractionStatus.MODIFIED);
145+
resource.setId(resourceId);
146+
}
135147

136148
if (logger.isLoggable(Level.FINE)) {
137149
logger.fine("Successfully inserted Resource. id=" + resource.getId() + " executionTime=" + dbCallDuration + "ms");
@@ -145,8 +157,6 @@ public Resource insert(Resource resource, List<ExtractedParameterValue> paramet
145157
if (FHIRDAOConstants.SQLSTATE_WRONG_VERSION.equals(e.getSQLState())) {
146158
// this is just a concurrency update, so there's no need to log the SQLException here
147159
throw new FHIRPersistenceVersionIdMismatchException("Encountered version id mismatch while inserting Resource");
148-
} else if (FHIRDAOConstants.SQLSTATE_MATCHES.equals(e.getSQLState())) {
149-
resource.setInteractionStatus(InteractionStatus.IF_NONE_MATCH_EXISTED);
150160
} else {
151161
FHIRPersistenceException fx = new FHIRPersistenceException("SQLException encountered while inserting Resource.");
152162
throw severe(logger, fx, e);
@@ -202,7 +212,8 @@ public Resource insert(Resource resource, List<ExtractedParameterValue> paramet
202212
public long storeResource(String tablePrefix, List<ExtractedParameterValue> parameters,
203213
String p_logical_id, InputStream p_payload, Timestamp p_last_updated, boolean p_is_deleted,
204214
String p_source_key, Integer p_version, String p_parameterHashB64, Connection conn,
205-
ParameterDAO parameterDao, Integer ifNoneMatch) throws Exception {
215+
ParameterDAO parameterDao, Integer ifNoneMatch,
216+
AtomicInteger outInteractionStatus, AtomicInteger outIfNoneMatchVersion) throws Exception {
206217

207218
final Calendar UTC = CalendarHelper.getCalendarForUTC();
208219
final String METHODNAME = "storeResource() for " + tablePrefix + " resource";
@@ -410,7 +421,9 @@ public long storeResource(String tablePrefix, List<ExtractedParameterValue> para
410421
if (!v_currently_deleted && checkIfNoneMatch(ifNoneMatch, v_current_version)) {
411422
// Conditional create-on-update If-None-Match matches the current resource, so skip the update
412423
logger.fine(() -> "Resource " + v_resource_type + "/" + p_logical_id + " [" + p_version + "] matches [If-None-Match: " + ifNoneMatch + "]");
413-
throw new SQLException("If-None-Match - record exists", FHIRDAOConstants.SQLSTATE_MATCHES);
424+
outInteractionStatus.set(1);
425+
outIfNoneMatchVersion.set(v_current_version);
426+
return -1L;
414427
} else if (p_version != v_current_version + 1) {
415428
// Concurrency check:
416429
// the version parameter we've been given (which is also embedded in the JSON payload) must be

‎fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dto/Resource.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ public class Resource {
6363
*/
6464
private InteractionStatus interactionStatus;
6565

66+
/**
67+
* The version of the resource found if we hit IfNoneMatch
68+
*/
69+
private Integer ifNoneMatchVersion;
6670

6771
public Resource() {
6872
super();
@@ -84,6 +88,22 @@ public void setLastUpdated(Timestamp lastUpdated) {
8488
this.lastUpdated = lastUpdated;
8589
}
8690

91+
/**
92+
* Setter for the ifNoneMatchVersion value
93+
* @param version
94+
*/
95+
public void setIfNoneMatchVersion(Integer version) {
96+
this.ifNoneMatchVersion = version;
97+
}
98+
99+
/**
100+
* Getter for the ifNoneMatchVersion value
101+
* @return
102+
*/
103+
public Integer getIfNoneMatchVersion() {
104+
return this.ifNoneMatchVersion;
105+
}
106+
87107
public long getId() {
88108
return id;
89109
}

‎fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/impl/FHIRPersistenceJDBCImpl.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,7 @@ public <T extends Resource> SingleResourceResult<T> createWithMeta(FHIRPersisten
416416
SingleResourceResult.Builder<T> resultBuilder = new SingleResourceResult.Builder<T>()
417417
.success(true)
418418
.interactionStatus(resourceDTO.getInteractionStatus())
419+
.ifNoneMatchVersion(resourceDTO.getIfNoneMatchVersion())
419420
.resource(updatedResource);
420421

421422
// Add supplemental issues to the OperationOutcome
@@ -606,6 +607,7 @@ public <T extends Resource> SingleResourceResult<T> updateWithMeta(FHIRPersisten
606607
SingleResourceResult.Builder<T> resultBuilder = new SingleResourceResult.Builder<T>()
607608
.success(true)
608609
.interactionStatus(resourceDTO.getInteractionStatus())
610+
.ifNoneMatchVersion(resourceDTO.getIfNoneMatchVersion())
609611
.resource(resource);
610612

611613
// Add supplemental issues to an OperationOutcome

‎fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/postgres/PostgresResourceDAO.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ public class PostgresResourceDAO extends ResourceDAOImpl {
5353

5454
private static final String SQL_READ_RESOURCE_TYPE = "{CALL %s.add_resource_type(?, ?)}";
5555

56-
// 12 args (9 in, 3 out)
57-
private static final String SQL_INSERT_WITH_PARAMETERS = "{CALL %s.add_any_resource(?,?,?,?,?,?,?,?,?,?,?,?)}";
56+
// 13 args (9 in, 4 out)
57+
private static final String SQL_INSERT_WITH_PARAMETERS = "{CALL %s.add_any_resource(?,?,?,?,?,?,?,?,?,?,?,?,?)}";
5858

5959
// DAO used to obtain sequence values from FHIR_REF_SEQUENCE
6060
private FhirRefSequenceDAO fhirRefSequenceDAO;
@@ -114,6 +114,7 @@ public Resource insert(Resource resource, List<ExtractedParameterValue> paramete
114114
stmt.registerOutParameter(10, Types.BIGINT);
115115
stmt.registerOutParameter(11, Types.VARCHAR); // The old parameter_hash
116116
stmt.registerOutParameter(12, Types.INTEGER); // o_interaction_status
117+
stmt.registerOutParameter(13, Types.INTEGER); // o_if_none_match_version
117118

118119
dbCallStartTime = System.nanoTime();
119120
stmt.execute();
@@ -124,6 +125,7 @@ public Resource insert(Resource resource, List<ExtractedParameterValue> paramete
124125
if (stmt.getInt(12) == 1) {
125126
// no change, so skip parameter updates
126127
resource.setInteractionStatus(InteractionStatus.IF_NONE_MATCH_EXISTED);
128+
resource.setIfNoneMatchVersion(stmt.getInt(13)); // current version
127129
} else {
128130
resource.setInteractionStatus(InteractionStatus.MODIFIED);
129131

‎fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/postgres/PostgresResourceNoProcDAO.java‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.util.Calendar;
1919
import java.util.List;
2020
import java.util.UUID;
21+
import java.util.concurrent.atomic.AtomicInteger;
2122
import java.util.logging.Level;
2223
import java.util.logging.Logger;
2324

@@ -107,6 +108,10 @@ public Resource insert(Resource resource, List<ExtractedParameterValue> paramete
107108
dbCallStartTime = System.nanoTime();
108109

109110
final String sourceKey = UUID.randomUUID().toString();
111+
112+
// to mimic out parameters from the stored procedure
113+
AtomicInteger outInteractionStatus = new AtomicInteger();
114+
AtomicInteger outIfNoneMatchVersion = new AtomicInteger();
110115

111116
long resourceId = this.storeResource(resource.getResourceType(),
112117
parameters,
@@ -119,14 +124,22 @@ public Resource insert(Resource resource, List<ExtractedParameterValue> paramete
119124
parameterHashB64,
120125
connection,
121126
parameterDao,
122-
ifNoneMatch
127+
ifNoneMatch,
128+
outInteractionStatus,
129+
outIfNoneMatchVersion
123130
);
124131

125132

126133
dbCallDuration = (System.nanoTime() - dbCallStartTime)/1e6;
127134

128-
resource.setId(resourceId);
129-
resource.setInteractionStatus(InteractionStatus.MODIFIED);
135+
if (outInteractionStatus.get() == 1) {
136+
resource.setInteractionStatus(InteractionStatus.IF_NONE_MATCH_EXISTED);
137+
resource.setIfNoneMatchVersion(outIfNoneMatchVersion.get());
138+
} else {
139+
resource.setInteractionStatus(InteractionStatus.MODIFIED);
140+
resource.setId(resourceId);
141+
}
142+
130143
if (logger.isLoggable(Level.FINE)) {
131144
logger.fine("Successfully inserted Resource. id=" + resource.getId() + " executionTime=" + dbCallDuration + "ms");
132145
}
@@ -139,8 +152,6 @@ public Resource insert(Resource resource, List<ExtractedParameterValue> paramete
139152
if (FHIRDAOConstants.SQLSTATE_WRONG_VERSION.equals(e.getSQLState())) {
140153
// this is just a concurrency update, so there's no need to log the SQLException here
141154
throw new FHIRPersistenceVersionIdMismatchException("Encountered version id mismatch while inserting Resource");
142-
} else if (FHIRDAOConstants.SQLSTATE_MATCHES.equals(e.getSQLState())) {
143-
resource.setInteractionStatus(InteractionStatus.IF_NONE_MATCH_EXISTED);
144155
} else {
145156
FHIRPersistenceException fx = new FHIRPersistenceException("SQLException encountered while inserting Resource.");
146157
throw severe(logger, fx, e);
@@ -194,7 +205,8 @@ public Resource insert(Resource resource, List<ExtractedParameterValue> paramete
194205
public long storeResource(String tablePrefix, List<ExtractedParameterValue> parameters, String p_logical_id,
195206
InputStream p_payload, Timestamp p_last_updated, boolean p_is_deleted,
196207
String p_source_key, Integer p_version, String parameterHashB64, Connection conn,
197-
ParameterDAO parameterDao, Integer ifNoneMatch) throws Exception {
208+
ParameterDAO parameterDao, Integer ifNoneMatch,
209+
AtomicInteger outInteractionStatus, AtomicInteger outIfNoneMatchVersion) throws Exception {
198210

199211
final String METHODNAME = "storeResource() for " + tablePrefix + " resource";
200212
logger.entering(CLASSNAME, METHODNAME);
@@ -330,7 +342,9 @@ public long storeResource(String tablePrefix, List<ExtractedParameterValue> para
330342
if (checkIfNoneMatch(ifNoneMatch, v_current_version)) {
331343
// Conditional create-on-update If-None-Match matches the current resource, so skip the update
332344
logger.fine(() -> "Resource " + v_resource_type + "/" + p_logical_id + " [" + p_version + "] matches [If-None-Match: " + ifNoneMatch + "]");
333-
throw new SQLException("If-None-Match - record exists", FHIRDAOConstants.SQLSTATE_MATCHES);
345+
outInteractionStatus.set(1);
346+
outIfNoneMatchVersion.set(v_current_version);
347+
return -1L;
334348
} else if (p_version != v_current_version + 1) {
335349
// Concurrency check:
336350
// the version parameter we've been given (which is also embedded in the JSON payload) must be

‎fhir-persistence-schema/src/main/resources/db2/add_any_resource.sql‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
-- o_logical_resource_id: output field returning the newly assigned logical_resource_id value
2727
-- o_resource_id: output field returning the newly assigned resource_id value
2828
-- o_current_parameter_hash: Base64 current parameter hash if existing resource
29+
-- o_interaction_status: output indicating whether a change was made or IfNoneMatch hit
30+
-- o_if_none_match_version: output revealing the version found when o_interaction_status is 1 (IfNoneMatch)
2931
-- Exceptions:
3032
-- SQLSTATE 99001: on version conflict (concurrency)
3133
-- SQLSTATE 99002: missing expected row (data integrity)
@@ -41,7 +43,8 @@
4143
OUT o_logical_resource_id BIGINT,
4244
OUT o_resource_row_id BIGINT,
4345
OUT o_current_parameter_hash VARCHAR(44 OCTETS),
44-
OUT o_interaction_status INT
46+
OUT o_interaction_status INT,
47+
OUT o_if_none_match_version INT
4548
)
4649
LANGUAGE SQL
4750
MODIFIES SQL DATA
@@ -142,6 +145,7 @@ BEGIN
142145
THEN
143146
-- If-None-Match: * hit
144147
SET o_interaction_status = 1;
148+
SET o_if_none_match_version = v_current_version;
145149
RETURN;
146150
END IF;
147151

‎fhir-persistence-schema/src/main/resources/postgres/add_any_resource.sql‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@
2121
-- p_version_id: the intended new version id of the resource (matching the JSON payload)
2222
-- p_parameter_hash_b64 the Base64 encoded hash of parameter values
2323
-- p_if_none_match the encoded If-None-Match value
24-
-- o_resource_id: output field returning the newly assigned resource_id value
24+
-- o_logical_resource_id: output field returning the newly assigned logical_resource_id value
25+
-- o_current_parameter_hash: Base64 current parameter hash if existing resource
26+
-- o_interaction_status: output indicating whether a change was made or IfNoneMatch hit
27+
-- o_if_none_match_version: output revealing the version found when o_interaction_status is 1 (IfNoneMatch)
2528
-- Exceptions:
2629
-- SQLSTATE 99001: on version conflict (concurrency)
2730
-- SQLSTATE 99002: missing expected row (data integrity)
@@ -37,7 +40,8 @@
3740
IN p_if_none_match INT,
3841
OUT o_logical_resource_id BIGINT,
3942
OUT o_current_parameter_hash VARCHAR( 44),
40-
OUT o_interaction_status INT)
43+
OUT o_interaction_status INT,
44+
OUT o_if_none_match_version INT)
4145
LANGUAGE plpgsql
4246
AS $$
4347

@@ -127,6 +131,7 @@ BEGIN
127131
-- connection with a fatal error, so instead we use an out parameter to
128132
-- indicate the match
129133
o_interaction_status := 1;
134+
o_if_none_match_version := v_current_version;
130135
RETURN;
131136
END IF;
132137

0 commit comments

Comments
 (0)