[GitHub] [incubator-eventmesh] JellyBo commented on a diff in pull request #921: [ISSUE #865] EM-Webhook: repair exception handling & optimize webHookConfig CURD logic

[GitBox <gi...@apache.org>]

JellyBo commented on code in PR #921:
URL: https://github.com/apache/incubator-eventmesh/pull/921#discussion_r900727681


##########
eventmesh-webhook/eventmesh-webhook-admin/src/main/java/org/apache/eventmesh/webhook/admin/FileWebHookConfigOperation.java:
##########
@@ -140,12 +117,50 @@ private WebHookConfig getWebHookConfigFromFile(File webhookConfigFile) {
 			while ((line = br.readLine()) != null) {
 				fileContent += line;
 			}
-		} catch (FileNotFoundException e) {
-			e.printStackTrace();
 		} catch (IOException e) {
-			e.printStackTrace();
+			logger.error("get webhook from file {} error", webhookConfigFile.getPath(), e);
 		}
 		return JsonUtils.deserialize(fileContent, WebHookConfig.class);
 	}
 
+	private synchronized boolean writeToFile(File webhookConfigFile, WebHookConfig webHookConfig) {
+		FileOutputStream fos = null;
+		BufferedWriter bw = null;
+		FileLock lock = null;
+		try {
+			fos = new FileOutputStream(webhookConfigFile);
+			bw = new BufferedWriter(new OutputStreamWriter(fos));
+			// lock this file
+			lock = fos.getChannel().lock();
+			bw.write(JsonUtils.serialize(webHookConfig));
+		} catch (IOException e) {
+			logger.error("write webhookConfig {} to file error", webHookConfig.getCallbackPath());
+			return false;
+		} finally {
+			try {
+				if (fos != null) {
+					fos.close();
+				}
+				if (bw != null) {
+					bw.close();
+				}
+				if (lock != null) {
+					lock.release();
+				}
+			} catch (IOException e) {
+				logger.warn("writeToFile finally caught an exception", e);
+			}
+		}
+		return true;
+	}
+
+	private String getWebhookConfigManuDir(WebHookConfig webHookConfig) {
+		return filePath + WebHookOperationConstant.FILE_SEPARATOR + webHookConfig.getCloudEventName();
+	}
+
+	private String getWebhookConfigFilePath(WebHookConfig webHookConfig) {
+		return this.getWebhookConfigManuDir(webHookConfig) + WebHookOperationConstant.FILE_SEPARATOR + MD5Utils.md5Hex(webHookConfig.getCallbackPath(), "UTF_8") + WebHookOperationConstant.FILE_EXTENSION;

Review Comment:
   because the path may contain some speacial char like '/', which is illegal as a file name. I've changed to use URLEncoder.encode method.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org