iframe-proxy

leavesster · 2026-01-31T09:27:33Z

Summary

Replace if-elif chain with COMPRESSION_SUFFIXES dictionary mapping
Use .get() to safely handle missing keys
Add unit tests for compression_suffix() function

Problem

Original code had long if-elif chain with potential KeyError risk:

if compression["method"] == "zip":
    return ".zip"
elif compression["method"] == "gzip":
    return ".gz"
# ... 6 more branches

Solution

COMPRESSION_SUFFIXES = {
    "zip": ".zip", "gzip": ".gz", "bz2": ".bz2",
    "zstd": ".zst", "xz": ".xz", "tar": ".tar",
}

def compression_suffix(context):
    compression = compression_options(context)
    if compression is None:
        return ".pkl"
    return COMPRESSION_SUFFIXES.get(compression.get("method"), ".pkl")

Test Plan

Added tests/test_serialization.py with coverage for all compression methods
All existing tests pass

Replace if-elif chain with a dictionary mapping for cleaner code. Also fix potential KeyError by using .get() method. Changes: - Add COMPRESSION_SUFFIXES constant mapping method to suffix - Use .get() to safely access 'method' key - Add comprehensive unit tests for compression_suffix

coderabbitai · 2026-01-31T09:27:49Z

Copilot

Pull request overview

Refactors compression_suffix() to use a dictionary mapping for compression method → file suffix (instead of an if/elif chain) and adds unit tests to validate behavior across supported and edge-case methods.

Changes:

Introduced COMPRESSION_SUFFIXES mapping and updated compression_suffix() to use .get()-based lookup with defaults.
Updated compression_suffix() docstring to match actual default behavior (.pkl).
Added unit tests covering supported methods and fallback behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`oocana/oocana/serialization.py`	Replaces if/elif suffix selection with a mapping constant and simplified lookup logic.
`oocana/tests/test_serialization.py`	Adds tests for `compression_suffix()` and `compression_options()` behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-31T09:30:46Z

@@ -0,0 +1,164 @@
+import unittest
+from unittest.mock import MagicMock, patch


patch is imported but never used in this test module; removing it avoids lint warnings and keeps the test file tidy.

Suggested change

from unittest.mock import MagicMock, patch

from unittest.mock import MagicMock

Copilot · 2026-01-31T09:30:46Z

+    method = compression.get("method")
+    if method is None:


COMPRESSION_SUFFIXES.get(method, ...) assumes method is a hashable key. If the options file contains a non-string (e.g., a list/dict from valid-but-unexpected JSON), this will raise TypeError and crash, whereas the previous if/elif chain would safely fall back. Consider guarding with isinstance(method, str) (and/or isinstance(compression, dict)) before doing the dict lookup, otherwise return the default suffix.

Suggested change

method = compression.get("method")

if method is None:

# Ensure we have a dictionary before accessing keys. Malformed or unexpected

# JSON in the options file may result in a non-dict value here.

if not isinstance(compression, dict):

return ".pkl"

method = compression.get("method")

# Guard against non-string (and thus potentially unhashable) methods.

if not isinstance(method, str):

Copilot · 2026-01-31T09:30:46Z

+# Mapping from compression method to file suffix
+COMPRESSION_SUFFIXES = {
+    "zip": ".zip",
+    "gzip": ".gz",
+    "bz2": ".bz2",
+    "zstd": ".zst",
+    "xz": ".xz",
+    "tar": ".tar",
+}


The compression methods are now duplicated in three places (SUPPORTED_COMPRESSION_METHODS, the CompressionOptions Literal, and COMPRESSION_SUFFIXES). This can drift over time (e.g., a method added to SUPPORTED_COMPRESSION_METHODS but missing here would silently fall back to .pkl). Consider making one source of truth (e.g., derive SUPPORTED_COMPRESSION_METHODS from COMPRESSION_SUFFIXES.keys()), and keep the type Literal in sync with that constant.

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@oocana/oocana/serialization.py`:
- Around line 87-96: compression_options 返回的 compression 可能不是 dict，从而在
compression.get("method") 处抛出 AttributeError；在函数/片段中（使用
compression_options、compression、method、COMPRESSION_SUFFIXES 的地方）先对 compression
做类型保护（例如 isinstance(compression, dict) 或
collections.abc.Mapping），若不是合适的映射类型则直接返回默认后缀 ".pkl"；若是映射再安全读取 method 并用
COMPRESSION_SUFFIXES.get(method, ".pkl") 返回结果。

🧹 Nitpick comments (2)

oocana/oocana/serialization.py (1)

72-80: 避免与 SUPPORTED_COMPRESSION_METHODS 重复来源，防止漂移
目前方法列表在 SUPPORTED_COMPRESSION_METHODS 和 COMPRESSION_SUFFIXES 中重复维护，后续新增/删除方法容易不同步。建议以 COMPRESSION_SUFFIXES 作为单一来源生成列表。若该常量希望对外公开，也可同步加入 __all__。
🔧 参考改法（统一来源）
-SUPPORTED_COMPRESSION_METHODS = ["zip", "gzip", "bz2", "zstd", "xz", "tar"]
+# Mapping from compression method to file suffix
+COMPRESSION_SUFFIXES = {
+    "zip": ".zip",
+    "gzip": ".gz",
+    "bz2": ".bz2",
+    "zstd": ".zst",
+    "xz": ".xz",
+    "tar": ".tar",
+}
+SUPPORTED_COMPRESSION_METHODS = list(COMPRESSION_SUFFIXES.keys())

oocana/tests/test_serialization.py (1)

29-119: 建议参数化这些重复的后缀测试
zip/gzip/bz2/zstd/xz/tar 的测试结构完全一致，后续维护成本偏高。可以用 subTest + 参数表合并为一个测试。

♻️ 参考改法（参数化）

-    def test_compression_suffix_zip(self):
-        """Test compression suffix for zip method."""
-        from oocana.oocana.serialization import compression_suffix, COMPRESSION_OPTIONS_FILE
-
-        with open(os.path.join(self.temp_dir, COMPRESSION_OPTIONS_FILE), "w") as f:
-            json.dump({"method": "zip"}, f)
-
-        result = compression_suffix(self.mock_context)
-        self.assertEqual(result, ".zip")
-
-    def test_compression_suffix_gzip(self):
-        """Test compression suffix for gzip method."""
-        from oocana.oocana.serialization import compression_suffix, COMPRESSION_OPTIONS_FILE
-
-        with open(os.path.join(self.temp_dir, COMPRESSION_OPTIONS_FILE), "w") as f:
-            json.dump({"method": "gzip"}, f)
-
-        result = compression_suffix(self.mock_context)
-        self.assertEqual(result, ".gz")
-
-    ...
-
-    def test_compression_suffix_tar(self):
-        """Test compression suffix for tar method."""
-        from oocana.oocana.serialization import compression_suffix, COMPRESSION_OPTIONS_FILE
-
-        with open(os.path.join(self.temp_dir, COMPRESSION_OPTIONS_FILE), "w") as f:
-            json.dump({"method": "tar"}, f)
-
-        result = compression_suffix(self.mock_context)
-        self.assertEqual(result, ".tar")
+    def test_compression_suffix_methods(self):
+        """Test compression suffix for supported methods."""
+        from oocana.oocana.serialization import compression_suffix, COMPRESSION_OPTIONS_FILE
+
+        cases = {
+            "zip": ".zip",
+            "gzip": ".gz",
+            "bz2": ".bz2",
+            "zstd": ".zst",
+            "xz": ".xz",
+            "tar": ".tar",
+        }
+        for method, expected in cases.items():
+            with self.subTest(method=method):
+                with open(os.path.join(self.temp_dir, COMPRESSION_OPTIONS_FILE), "w") as f:
+                    json.dump({"method": method}, f)
+                self.assertEqual(compression_suffix(self.mock_context), expected)

coderabbitai · 2026-01-31T09:31:11Z

    compression = compression_options(context)

-    if compression is None or compression["method"] is None:
+    if compression is None:
        return ".pkl"
-
-    method = compression["method"]
-    if method == "zip":
-        return ".zip"
-    elif method == "gzip":
-        return ".gz"
-    elif method == "bz2":
-        return ".bz2"
-    elif method == "zstd":
-        return ".zst"
-    elif method == "xz":
-        return ".xz"
-    elif method == "tar":
-        return ".tar"
-    else:
-        return ".pkl"  # Default case if method is not recognized
+
+    method = compression.get("method")
+    if method is None:
+        return ".pkl"
+
+    return COMPRESSION_SUFFIXES.get(method, ".pkl")


⚠️ Potential issue | 🟡 Minor

防止压缩配置为非 dict 时触发 AttributeError
compression_options 读取的是任意 JSON，若文件内容为合法但非 dict（如字符串/列表），compression.get(...) 会直接报错。建议加类型保护并回退到默认后缀。

🛡️ 参考修复

compression = compression_options(context) - if compression is None: + if not isinstance(compression, dict): return ".pkl"

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

compression = compression_options(context)

if compression is None or compression["method"] is None:

if compression is None:

return ".pkl"

method = compression["method"]

if method == "zip":

return ".zip"

elif method == "gzip":

return ".gz"

elif method == "bz2":

return ".bz2"

elif method == "zstd":

return ".zst"

elif method == "xz":

return ".xz"

elif method == "tar":

return ".tar"

else:

return ".pkl" # Default case if method is not recognized

\ No newline at end of file

method = compression.get("method")

if method is None:

return ".pkl"

return COMPRESSION_SUFFIXES.get(method, ".pkl")

compression = compression_options(context)

if not isinstance(compression, dict):

return ".pkl"

method = compression.get("method")

if method is None:

return ".pkl"

return COMPRESSION_SUFFIXES.get(method, ".pkl")

🤖 Prompt for AI Agents

In `@oocana/oocana/serialization.py` around lines 87 - 96, compression_options 返回的 compression 可能不是 dict，从而在 compression.get("method") 处抛出 AttributeError；在函数/片段中（使用 compression_options、compression、method、COMPRESSION_SUFFIXES 的地方）先对 compression 做类型保护（例如 isinstance(compression, dict) 或 collections.abc.Mapping），若不是合适的映射类型则直接返回默认后缀 ".pkl"；若是映射再安全读取 method 并用 COMPRESSION_SUFFIXES.get(method, ".pkl") 返回结果。

Copilot AI review requested due to automatic review settings January 31, 2026 09:27

Copilot started reviewing on behalf of leavesster January 31, 2026 09:27 View session

Copilot AI reviewed Jan 31, 2026

View reviewed changes

coderabbitai Bot reviewed Jan 31, 2026

View reviewed changes

leavesster changed the title ~~fix: use dict mapping for compression_suffix instead of if-elif chain~~ fix(oocana): use dict mapping for compression_suffix instead of if-elif chain Jan 31, 2026

Sunbelt Computer Software

PL/B Language Development and Support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(oocana): use dict mapping for compression_suffix instead of if-elif chain#457

fix(oocana): use dict mapping for compression_suffix instead of if-elif chain#457
leavesster wants to merge 1 commit into
mainfrom
fix/compression-suffix-dict-mapping

leavesster commented Jan 31, 2026

Uh oh!

coderabbitai Bot commented Jan 31, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 31, 2026

Uh oh!

Copilot AI Jan 31, 2026

Uh oh!

Copilot AI Jan 31, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Jan 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1,164 @@
		import unittest
		from unittest.mock import MagicMock, patch

	from unittest.mock import MagicMock, patch
	from unittest.mock import MagicMock

-    method = compression.get("method")
-    if method is None:
+    # Ensure we have a dictionary before accessing keys. Malformed or unexpected
+    # JSON in the options file may result in a non-dict value here.
+    if not isinstance(compression, dict):
+        return ".pkl"
+    method = compression.get("method")
+    # Guard against non-string (and thus potentially unhashable) methods.
+    if not isinstance(method, str):

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

leavesster commented Jan 31, 2026

Summary

Problem

Solution

Test Plan

Uh oh!

coderabbitai Bot commented Jan 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

总体说明

变更内容

代码审查工作量估算

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jan 31, 2026 •

edited

Loading