[ISSUE #4661]configServletInner#doGetConfig code optimization (#4666)

* configServletInner#doGetConfig code optimization * add comment * update comment * update comment
alibaba · Jan 15, 2021 · 63513b4 · 63513b4
1 parent 1bd6726
commit 63513b4
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 33 deletions.
diff --git a/config/src/main/java/com/alibaba/nacos/config/server/controller/ConfigServletInner.java b/config/src/main/java/com/alibaba/nacos/config/server/controller/ConfigServletInner.java
@@ -128,25 +128,23 @@ public String doGetConfig(HttpServletRequest request, HttpServletResponse respon
         final String requestIp = RequestUtil.getRemoteIp(request);
         boolean isBeta = false;
         if (lockResult > 0) {
+            // LockResult > 0 means cacheItem is not null and other thread can`t delete this cacheItem
             FileInputStream fis = null;
             try {
                 String md5 = Constants.NULL;
                 long lastModified = 0L;
                 CacheItem cacheItem = ConfigCacheService.getContentCache(groupKey);
-                if (cacheItem != null) {
-                    if (cacheItem.isBeta()) {
-                        if (cacheItem.getIps4Beta().contains(clientIp)) {
-                            isBeta = true;
-                        }
-                    }
-
-                    final String configType =
-                            (null != cacheItem.getType()) ? cacheItem.getType() : FileTypeEnum.TEXT.getFileType();
-                    response.setHeader("Config-Type", configType);
-                    FileTypeEnum fileTypeEnum = FileTypeEnum.getFileTypeEnumByFileExtensionOrFileType(configType);
-                    String contentTypeHeader = fileTypeEnum.getContentType();
-                    response.setHeader(HttpHeaderConsts.CONTENT_TYPE, contentTypeHeader);
+                if (cacheItem.isBeta() && cacheItem.getIps4Beta().contains(clientIp)) {
+                    isBeta = true;
                 }
+
+                final String configType =
+                        (null != cacheItem.getType()) ? cacheItem.getType() : FileTypeEnum.TEXT.getFileType();
+                response.setHeader("Config-Type", configType);
+                FileTypeEnum fileTypeEnum = FileTypeEnum.getFileTypeEnumByFileExtensionOrFileType(configType);
+                String contentTypeHeader = fileTypeEnum.getContentType();
+                response.setHeader(HttpHeaderConsts.CONTENT_TYPE, contentTypeHeader);
+
                 File file = null;
                 ConfigInfoBase configInfoBase = null;
                 PrintWriter out = null;
@@ -162,13 +160,11 @@ public String doGetConfig(HttpServletRequest request, HttpServletResponse respon
                 } else {
                     if (StringUtils.isBlank(tag)) {
                         if (isUseTag(cacheItem, autoTag)) {
-                            if (cacheItem != null) {
-                                if (cacheItem.tagMd5 != null) {
-                                    md5 = cacheItem.tagMd5.get(autoTag);
-                                }
-                                if (cacheItem.tagLastModifiedTs != null) {
-                                    lastModified = cacheItem.tagLastModifiedTs.get(autoTag);
-                                }
+                            if (cacheItem.tagMd5 != null) {
+                                md5 = cacheItem.tagMd5.get(autoTag);
+                            }
+                            if (cacheItem.tagLastModifiedTs != null) {
+                                lastModified = cacheItem.tagLastModifiedTs.get(autoTag);
                             }
                             if (PropertyUtil.isDirectRead()) {
                                 configInfoBase = persistService.findConfigInfo4Tag(dataId, group, tenant, autoTag);
@@ -202,15 +198,13 @@ public String doGetConfig(HttpServletRequest request, HttpServletResponse respon
                             }
                         }
                     } else {
-                        if (cacheItem != null) {
-                            if (cacheItem.tagMd5 != null) {
-                                md5 = cacheItem.tagMd5.get(tag);
-                            }
-                            if (cacheItem.tagLastModifiedTs != null) {
-                                Long lm = cacheItem.tagLastModifiedTs.get(tag);
-                                if (lm != null) {
-                                    lastModified = lm;
-                                }
+                        if (cacheItem.tagMd5 != null) {
+                            md5 = cacheItem.tagMd5.get(tag);
+                        }
+                        if (cacheItem.tagLastModifiedTs != null) {
+                            Long lm = cacheItem.tagLastModifiedTs.get(tag);
+                            if (lm != null) {
+                                lastModified = lm;
                             }
                         }
                         if (PropertyUtil.isDirectRead()) {
@@ -301,7 +295,12 @@ public String doGetConfig(HttpServletRequest request, HttpServletResponse respon
     private static void releaseConfigReadLock(String groupKey) {
         ConfigCacheService.releaseReadLock(groupKey);
     }
-
+
+    /**
+     * Try to add read lock.
+     * @param groupKey groupKey string value.
+     * @return 0 - No data and failed. Positive number - lock succeeded. Negative number - lock failed。
+     */
     private static int tryConfigReadLock(String groupKey) {
 
         // Lock failed by default.

diff --git a/config/src/main/java/com/alibaba/nacos/config/server/service/ConfigCacheService.java b/config/src/main/java/com/alibaba/nacos/config/server/service/ConfigCacheService.java
@@ -607,11 +607,11 @@ public static boolean isUptodate(String groupKey, String md5, String ip, String
     }
 
     /**
-     * Try to add read lock. If it successed, then it can call {@link #releaseWriteLock(String)}.And it won't call if
+     * Try to add read lock. If it succeeded, then it can call {@link #releaseWriteLock(String)}.And it won't call if
      * failed.
      *
      * @param groupKey groupKey string value.
-     * @return 0 - No data and failed. Positive number 0 - Success. Negative number - lock failed。
+     * @return 0 - No data and failed. Positive number - lock succeeded. Negative number - lock failed。
      */
     public static int tryReadLock(String groupKey) {
         CacheItem groupItem = CACHE.get(groupKey);
@@ -635,7 +635,7 @@ public static void releaseReadLock(String groupKey) {
     }
 
     /**
-     * Try to add write lock. If it successed, then it can call {@link #releaseWriteLock(String)}.And it won't call if
+     * Try to add write lock. If it succeeded, then it can call {@link #releaseWriteLock(String)}.And it won't call if
      * failed.
      *
      * @param groupKey groupKey string value.