diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d5a393..b7b7c5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [3.0.2] - 2026-09-04 + +### Fixed + +- **A successful CX2 network update was reported as a failure.** `updateCX2Network` treated only `204 No Content` as success, but `PUT /v3/networks/{networkid}` answers `200` with an `NdexObjectUpdateStatus` body. A successful v3 update therefore took the error path, where its own success body failed to parse as an `NDExError` and surfaced as `Unrecognized field "uuid"` — on an update the server had already applied, making it a false failure a caller would retry. Any 2xx is now treated as success, so both the v2 (`204`) and v3 (`200`) contracts are honoured. +- **Update errors are read from the error stream.** The same method read the response body with `getInputStream()`, which throws for a 4xx or 5xx, so a genuine server error surfaced as an `IOException` rather than the `NdexException` the body describes. It now reads `getErrorStream()`, matching every other method in the client. + ## [3.0.1] - 2026-09-03 ### Added diff --git a/pom.xml b/pom.xml index 5ba99a5..ade6251 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ 4.0.0 org.ndexbio.client ndex-java-client - 3.0.1 + 3.0.2 NDEx Java REST Client REST Client for in applications that access NDEx 2013 diff --git a/src/main/java/org/ndexbio/rest/client/NdexRestClientModelAccessLayer.java b/src/main/java/org/ndexbio/rest/client/NdexRestClientModelAccessLayer.java index da7bb1c..4ab6c4d 100644 --- a/src/main/java/org/ndexbio/rest/client/NdexRestClientModelAccessLayer.java +++ b/src/main/java/org/ndexbio/rest/client/NdexRestClientModelAccessLayer.java @@ -889,8 +889,16 @@ private void updateNetwork(UUID networkUUID, InputStream input, final String rou HttpURLConnection con = ndexRestClient.createReturningConnection(route + "/" + networkUUID.toString() + query, input, "PUT", jsonAcceptContentRequestProperties); - if (con.getResponseCode() != HttpURLConnection.HTTP_NO_CONTENT){ - ndexRestClient.processNdexSpecificException(con.getInputStream(), con.getResponseCode(), new ObjectMapper()); + final int status = con.getResponseCode(); + // Any 2xx is a success. The v2 endpoint answers 204 No Content, but v3 answers 200 with an + // NdexObjectUpdateStatus body -- so checking only for 204 sent a successful v3 update down the + // error path, where the success body failed to parse as an NDExError and surfaced as + // 'Unrecognized field "uuid"' on an update that had in fact already been applied. + if (status < HttpURLConnection.HTTP_OK || status >= HttpURLConnection.HTTP_MULT_CHOICE) { + // Error bodies arrive on the error stream; getInputStream() throws for 4xx and 5xx. + try (InputStream errorBody = con.getErrorStream()) { + ndexRestClient.processNdexSpecificException(errorBody, status, new ObjectMapper()); + } } } diff --git a/src/test/java/org/ndexbio/rest/client/NdexRestClientModelAccessLayerV3Test.java b/src/test/java/org/ndexbio/rest/client/NdexRestClientModelAccessLayerV3Test.java index d71dd8b..cdb5e4d 100644 --- a/src/test/java/org/ndexbio/rest/client/NdexRestClientModelAccessLayerV3Test.java +++ b/src/test/java/org/ndexbio/rest/client/NdexRestClientModelAccessLayerV3Test.java @@ -8,6 +8,7 @@ import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @@ -254,6 +255,53 @@ private String captureUpdateRoute(VisibilityType visibility) throws Exception { return route.getValue(); } + @Test + public void updateSucceedsWhenV3AnswersTwoHundredWithABody() throws Exception { + // The bug this guards: only 204 was treated as success, so a successful v3 update took the error + // path and failed parsing its own success body as an NDExError. + NdexRestClient client = mock(NdexRestClient.class); + expect(client.createReturningConnection(anyObject(String.class), anyObject(InputStream.class), eq("PUT"), + anyObject())).andReturn(okWithBodyConnection()); + replay(client); + + new NdexRestClientModelAccessLayer(client).updateCX2Network(NETWORK_ID, cx2Stream(), + VisibilityType.PUBLIC); + + verify(client); + } + + @Test + public void updateStillSucceedsWhenV2AnswersNoContent() throws Exception { + NdexRestClient client = mock(NdexRestClient.class); + expect(client.createReturningConnection(anyObject(String.class), anyObject(InputStream.class), eq("PUT"), + anyObject())).andReturn(noContentConnection()); + replay(client); + + new NdexRestClientModelAccessLayer(client).updateCXNetwork(NETWORK_ID, cx2Stream()); + + verify(client); + } + + @Test + public void updateReportsAServerErrorFromTheErrorStream() throws Exception { + // getInputStream() throws for a 4xx, so the error body has to come off getErrorStream() + NdexRestClient client = mock(NdexRestClient.class); + expect(client.createReturningConnection(anyObject(String.class), anyObject(InputStream.class), eq("PUT"), + anyObject())).andReturn(notFoundConnection()); + client.processNdexSpecificException(anyObject(InputStream.class), eq(HttpURLConnection.HTTP_NOT_FOUND), + anyObject(com.fasterxml.jackson.databind.ObjectMapper.class)); + expectLastCall().andThrow(new org.ndexbio.model.exceptions.ObjectNotFoundException("network", NETWORK_ID.toString())); + replay(client); + + try { + new NdexRestClientModelAccessLayer(client).updateCX2Network(NETWORK_ID, cx2Stream(), null); + fail("expected the server error to surface"); + } catch (org.ndexbio.model.exceptions.NdexException expected) { + // the point is that it is an NdexException, not a Jackson parse failure + } + verify(client); + } + // ---------- helpers ---------- private static InputStream cx2Stream() { @@ -269,6 +317,29 @@ private static HttpURLConnection createdConnection(UUID newId) throws Exception return con; } + /** What v3 actually answers to PUT /v3/networks/{id}: 200 with an NdexObjectUpdateStatus body. */ + private static HttpURLConnection okWithBodyConnection() throws Exception { + HttpURLConnection con = mock(HttpURLConnection.class); + expect(con.getResponseCode()).andReturn(HttpURLConnection.HTTP_OK).anyTimes(); + expect(con.getInputStream()).andReturn(new ByteArrayInputStream( + ("{\"uuid\":\"" + NETWORK_ID + "\",\"modificationTime\":\"2026-09-04T00:00:00Z\"}") + .getBytes(StandardCharsets.UTF_8))).anyTimes(); + expect(con.getErrorStream()).andReturn(null).anyTimes(); + replay(con); + return con; + } + + /** A failing update: an NDExError body on the error stream. */ + private static HttpURLConnection notFoundConnection() throws Exception { + HttpURLConnection con = mock(HttpURLConnection.class); + expect(con.getResponseCode()).andReturn(HttpURLConnection.HTTP_NOT_FOUND).anyTimes(); + expect(con.getErrorStream()).andReturn(new ByteArrayInputStream( + "{\"errorCode\":\"NDEx_Object_Not_Found\",\"message\":\"no such network\"}" + .getBytes(StandardCharsets.UTF_8))).anyTimes(); + replay(con); + return con; + } + private static HttpURLConnection noContentConnection() throws Exception { HttpURLConnection con = mock(HttpURLConnection.class); expect(con.getResponseCode()).andReturn(HttpURLConnection.HTTP_NO_CONTENT).anyTimes();