From dad6cae3802181a191bbfa01498f6a765f3378bc Mon Sep 17 00:00:00 2001 From: Hiroyuki Wada Date: Fri, 9 Aug 2024 11:22:54 +0900 Subject: [PATCH] fix: error handling for GET request --- .../connector/notion/NotionRESTClient.java | 78 +++++++++---------- .../connector/util/AbstractRESTClient.java | 67 +++++++++++++--- .../connector/notion/UserTest.java | 22 ++++++ 3 files changed, 116 insertions(+), 51 deletions(-) diff --git a/src/main/java/jp/openstandia/connector/notion/NotionRESTClient.java b/src/main/java/jp/openstandia/connector/notion/NotionRESTClient.java index 049f8b6..7a6f100 100644 --- a/src/main/java/jp/openstandia/connector/notion/NotionRESTClient.java +++ b/src/main/java/jp/openstandia/connector/notion/NotionRESTClient.java @@ -137,14 +137,15 @@ public Uid createUser(NotionUserModel newUser) throws AlreadyExistsException { } public NotionUserModel getUser(Uid uid, OperationOptions options, Set fetchFieldsSet) throws UnknownUidException { - try (Response response = get(userEndpoint + "/" + uid.getUidValue(), null)) { - try { - NotionUserModel user = MAPPER.readValue(response.body().byteStream(), NotionUserModel.class); - return user; - - } catch (IOException e) { - throw new ConnectorIOException(String.format("Cannot parse %s REST API Response", instanceName), e); + try (Response response = callRead(USER_OBJECT_CLASS, userEndpoint, uid)) { + if (response == null) { + return null; } + NotionUserModel user = MAPPER.readValue(response.body().byteStream(), NotionUserModel.class); + return user; + + } catch (IOException e) { + throw new ConnectorIOException(String.format("Cannot parse %s REST API Response", instanceName), e); } } @@ -152,18 +153,16 @@ public NotionUserModel getUser(Name name, OperationOptions options, Set Map params = new HashMap<>(); params.put("filter", formatFilter("userName eq \"%s\"", name.getNameValue())); - try (Response response = get(userEndpoint, params)) { - try { - UserListBody list = MAPPER.readValue(response.body().byteStream(), UserListBody.class); - if (list.resources == null || list.resources.size() != 1) { - LOG.info("The user is not found. userName={0}", name.getNameValue()); - return null; - } - return list.resources.get(0); - - } catch (IOException e) { - throw new ConnectorIOException(String.format("Cannot parse %s REST API Response", instanceName), e); + try (Response response = callSearch(USER_OBJECT_CLASS, userEndpoint, params)) { + UserListBody list = MAPPER.readValue(response.body().byteStream(), UserListBody.class); + if (list.resources == null || list.resources.size() != 1) { + LOG.info("The {0} user is not found. userName={1}", instanceName, name.getNameValue()); + return null; } + return list.resources.get(0); + + } catch (IOException e) { + throw new ConnectorIOException(String.format("Cannot parse %s REST API Response", instanceName), e); } } @@ -190,7 +189,7 @@ public int getUsers(QueryHandler handler, OperationOptions opti params.put(offsetKey, String.valueOf(start)); params.put(countKey, String.valueOf(size)); - try (Response response = get(userEndpoint, params)) { + try (Response response = callSearch(USER_OBJECT_CLASS, userEndpoint, params)) { UserListBody list = MAPPER.readValue(response.body().byteStream(), UserListBody.class); return list.resources; @@ -208,7 +207,7 @@ public int getUsers(QueryHandler handler, OperationOptions opti params.put(offsetKey, String.valueOf(start)); params.put(countKey, String.valueOf(pageSize)); - try (Response response = get(userEndpoint, params)) { + try (Response response = callSearch(USER_OBJECT_CLASS, userEndpoint, params)) { UserListBody list = MAPPER.readValue(response.body().byteStream(), UserListBody.class); for (NotionUserModel user : list.resources) { if (!handler.handle(user)) { @@ -241,14 +240,15 @@ public void patchGroup(Uid uid, PatchOperationsModel operations) { } public NotionGroupModel getGroup(Uid uid, OperationOptions options, Set fetchFieldsSet) throws UnknownUidException { - try (Response response = get(groupEndpoint + "/" + uid.getUidValue(), null)) { - try { - NotionGroupModel group = MAPPER.readValue(response.body().byteStream(), NotionGroupModel.class); - return group; - - } catch (IOException e) { - throw new ConnectorIOException(String.format("Cannot parse %s REST API Response", instanceName), e); + try (Response response = callRead(GROUP_OBJECT_CLASS, groupEndpoint, uid)) { + if (response == null) { + return null; } + NotionGroupModel group = MAPPER.readValue(response.body().byteStream(), NotionGroupModel.class); + return group; + + } catch (IOException e) { + throw new ConnectorIOException(String.format("Cannot parse %s REST API Response", instanceName), e); } } @@ -256,18 +256,16 @@ public NotionGroupModel getGroup(Name name, OperationOptions options, Set params = new HashMap<>(); params.put("filter", formatFilter("displayName eq \"%s\"", name.getNameValue())); - try (Response response = get(groupEndpoint, params)) { - try { - GroupListBody list = MAPPER.readValue(response.body().byteStream(), GroupListBody.class); - if (list.resources == null || list.resources.size() != 1) { - LOG.info("The group is not found. displayName={0}", name.getNameValue()); - return null; - } - return list.resources.get(0); - - } catch (IOException e) { - throw new ConnectorIOException(String.format("Cannot parse %s REST API Response", instanceName), e); + try (Response response = callSearch(GROUP_OBJECT_CLASS, groupEndpoint, params)) { + GroupListBody list = MAPPER.readValue(response.body().byteStream(), GroupListBody.class); + if (list.resources == null || list.resources.size() != 1) { + LOG.info("The {0} group is not found. displayName={1}", instanceName, name.getNameValue()); + return null; } + return list.resources.get(0); + + } catch (IOException e) { + throw new ConnectorIOException(String.format("Cannot parse %s REST API Response", instanceName), e); } } @@ -279,7 +277,7 @@ public int getGroups(QueryHandler handler, OperationOptions op params.put(offsetKey, String.valueOf(start)); params.put(countKey, String.valueOf(size)); - try (Response response = get(groupEndpoint, params)) { + try (Response response = callSearch(GROUP_OBJECT_CLASS, groupEndpoint, params)) { GroupListBody list = MAPPER.readValue(response.body().byteStream(), GroupListBody.class); return list.resources; @@ -296,7 +294,7 @@ public int getGroups(QueryHandler handler, OperationOptions op params.put(offsetKey, String.valueOf(start)); params.put(countKey, String.valueOf(pageSize)); - try (Response response = get(groupEndpoint, params)) { + try (Response response = callSearch(GROUP_OBJECT_CLASS, groupEndpoint, params)) { GroupListBody list = MAPPER.readValue(response.body().byteStream(), GroupListBody.class); for (NotionGroupModel group : list.resources) { if (!handler.handle(group)) { diff --git a/src/main/java/jp/openstandia/connector/util/AbstractRESTClient.java b/src/main/java/jp/openstandia/connector/util/AbstractRESTClient.java index 570a018..0b92266 100644 --- a/src/main/java/jp/openstandia/connector/util/AbstractRESTClient.java +++ b/src/main/java/jp/openstandia/connector/util/AbstractRESTClient.java @@ -97,7 +97,7 @@ protected T callCreate(ObjectClass objectClass, String url, Object target, S throw new AlreadyExistsException(String.format("%s %s '%s' already exists.", instanceName, objectClass.getObjectClassValue(), name)); } if (errorHandler.isInvalidRequest(response)) { - throw new InvalidAttributeValueException(String.format("Bad request when creating %s %s '%s': %s", instanceName, objectClass.getObjectClassValue(), name, toBody(response))); + throw new InvalidAttributeValueException(String.format("Bad request in create operation %s %s '%s': %s", instanceName, objectClass.getObjectClassValue(), name, toBody(response))); } if (!this.errorHandler.isOk(response)) { @@ -121,7 +121,7 @@ protected void callPatch(ObjectClass objectClass, String url, Uid uid, Object ta } if (this.errorHandler.isInvalidRequest(response)) { - throw new InvalidAttributeValueException(String.format("Bad request when updating %s %s: %s, response: %s", + throw new InvalidAttributeValueException(String.format("Bad request in update operation %s %s: %s, response: %s", this.instanceName, objectClass.getObjectClassValue(), uid.getUidValue(), toBody(response))); } @@ -145,7 +145,7 @@ protected void callUpdate(ObjectClass objectClass, String url, Uid uid, Object t } if (this.errorHandler.isInvalidRequest(response)) { - throw new InvalidAttributeValueException(String.format("Bad request when updating %s %s: %s, response: %s", + throw new InvalidAttributeValueException(String.format("Bad request in replace operation %s %s: %s, response: %s", this.instanceName, objectClass.getObjectClassValue(), uid.getUidValue(), toBody(response))); } @@ -192,7 +192,7 @@ protected void callDelete(ObjectClass objectClass, String url, Uid uid, Object b } if (this.errorHandler.isInvalidRequest(response)) { - throw new InvalidAttributeValueException(String.format("Bad request when deleting %s %s: %s, response: %s", + throw new InvalidAttributeValueException(String.format("Bad request in delete operation %s %s: %s, response: %s", this.instanceName, objectClass.getObjectClassValue(), uid.getUidValue(), toBody(response))); } @@ -209,6 +209,55 @@ protected void callDelete(ObjectClass objectClass, String url, Uid uid, Object b } } + protected Response callRead(ObjectClass objectClass, String url, Uid uid) { + try { + Response response = get(url + "/" + uid.getUidValue()); + if (this.errorHandler.isNotFound(response)) { + // Don't return UnknownUidException in the Search (executeQuery) operations + return null; + } + + if (this.errorHandler.isInvalidRequest(response)) { + throw new InvalidAttributeValueException(String.format("Bad request in read operation for %s %s: %s, response: %s", + this.instanceName, objectClass.getObjectClassValue(), uid.getUidValue(), toBody(response))); + } + + if (!this.errorHandler.isOk(response)) { + throw new ConnectorIOException(String.format("Failed to read %s %s: %s, statusCode: %d, response: %s", + this.instanceName, objectClass.getObjectClassValue(), uid.getUidValue(), response.code(), toBody(response))); + } + + // Success + return response; + + } catch (IOException e) { + throw new ConnectorIOException(String.format("Failed to read %s %s: %s", + this.instanceName, objectClass.getObjectClassValue(), uid.getUidValue()), e); + } + } + + protected Response callSearch(ObjectClass objectClass, String url, Map params) { + try { + Response response = get(url, params); + if (this.errorHandler.isInvalidRequest(response)) { + throw new InvalidAttributeValueException(String.format("Bad request in search operation for %s %s: %s, response: %s", + this.instanceName, objectClass.getObjectClassValue(), params, toBody(response))); + } + + if (!this.errorHandler.isOk(response)) { + throw new ConnectorIOException(String.format("Failed to search %s %s: %s, statusCode: %d, response: %s", + this.instanceName, objectClass.getObjectClassValue(), params, response.code(), toBody(response))); + } + + // Success + return response; + + } catch (IOException e) { + throw new ConnectorIOException(String.format("Failed to search %s %s: %s", + this.instanceName, objectClass.getObjectClassValue(), params), e); + } + } + private RequestBody createJsonRequestBody(Object body) { String bodyString; try { @@ -238,11 +287,11 @@ private void throwExceptionIfServerError(Response response) throws ConnectorIOEx } } - protected Response get(String url) throws ConnectorIOException { + protected Response get(String url) throws IOException { return get(url, null); } - protected Response get(String url, Map params) throws ConnectorIOException { + protected Response get(String url, Map params) throws IOException { HttpUrl.Builder httpBuilder = HttpUrl.parse(url).newBuilder(); if (params != null) { @@ -255,11 +304,7 @@ protected Response get(String url, Map params) throws ConnectorI .build(); final Response response; - try { - response = httpClient.newCall(request).execute(); - } catch (IOException e) { - throw new ConnectorIOException(this.instanceName + " server error", e); - } + response = httpClient.newCall(request).execute(); throwExceptionIfUnauthorized(response); throwExceptionIfServerError(response); diff --git a/src/test/java/jp/openstandia/connector/notion/UserTest.java b/src/test/java/jp/openstandia/connector/notion/UserTest.java index 813ca1b..544af89 100644 --- a/src/test/java/jp/openstandia/connector/notion/UserTest.java +++ b/src/test/java/jp/openstandia/connector/notion/UserTest.java @@ -292,6 +292,28 @@ void getUserByUidWithEmpty() { assertNull(result.getAttributeByName("name.givenName")); } + @Test + void getUserByUidWithNotFound() { + ConnectorFacade connector = newFacade(configuration); + + // Given + String userId = "12345"; + String userName = "foo"; + + AtomicReference targetUid = new AtomicReference<>(); + mockClient.getUserByUid = ((u) -> { + targetUid.set(u); + return null; + }); + + // When + ConnectorObject result = connector.getObject(USER_OBJECT_CLASS, new Uid(userId, new Name(userName)), defaultGetOperation()); + + // Then + assertNull(result); + assertNotNull(targetUid.get()); + } + @Test void getUserByName() { // Given