Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions oocana/oocana/serialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,28 +69,28 @@ def compression_options(context: 'Context') -> CompressionOptions | None:
print(f"An unexpected error occurred while reading compression options: {e}. Returning None.")
return None

# Mapping from compression method to file suffix
COMPRESSION_SUFFIXES = {
"zip": ".zip",
"gzip": ".gz",
"bz2": ".bz2",
"zstd": ".zst",
"xz": ".xz",
"tar": ".tar",
}
Comment on lines +72 to +80
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

def compression_suffix(context: 'Context') -> str:
"""
Get the file suffix based on the compression method.
If no compression is specified, return an empty string.
If no compression is specified, return ".pkl" (pickle format).
"""
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:
Comment on lines +92 to +93
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
return ".pkl"

return COMPRESSION_SUFFIXES.get(method, ".pkl")
Comment on lines 87 to 96
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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") 返回结果。

164 changes: 164 additions & 0 deletions oocana/tests/test_serialization.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
import unittest
from unittest.mock import MagicMock, patch
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
import tempfile
import os
import json


class TestCompressionSuffix(unittest.TestCase):
"""Test cases for compression_suffix function."""

def setUp(self):
# Create a temporary directory for the mock context
self.temp_dir = tempfile.mkdtemp()
self.mock_context = MagicMock()
self.mock_context.pkg_data_dir = self.temp_dir

def tearDown(self):
# Clean up temporary files
import shutil
shutil.rmtree(self.temp_dir, ignore_errors=True)

def test_compression_suffix_no_options(self):
"""Test that .pkl is returned when no compression options exist."""
from oocana.oocana.serialization import compression_suffix

result = compression_suffix(self.mock_context)
self.assertEqual(result, ".pkl")

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_bz2(self):
"""Test compression suffix for bz2 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": "bz2"}, f)

result = compression_suffix(self.mock_context)
self.assertEqual(result, ".bz2")

def test_compression_suffix_zstd(self):
"""Test compression suffix for zstd 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": "zstd"}, f)

result = compression_suffix(self.mock_context)
self.assertEqual(result, ".zst")

def test_compression_suffix_xz(self):
"""Test compression suffix for xz 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": "xz"}, f)

result = compression_suffix(self.mock_context)
self.assertEqual(result, ".xz")

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_null_method(self):
"""Test that .pkl is returned when method is null."""
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": None}, f)

result = compression_suffix(self.mock_context)
self.assertEqual(result, ".pkl")

def test_compression_suffix_missing_method_key(self):
"""Test that .pkl is returned when method key is missing (no KeyError)."""
from oocana.oocana.serialization import compression_suffix, COMPRESSION_OPTIONS_FILE

# Write a dict without the 'method' key
with open(os.path.join(self.temp_dir, COMPRESSION_OPTIONS_FILE), "w") as f:
json.dump({"other_key": "value"}, f)

# Should not raise KeyError
result = compression_suffix(self.mock_context)
self.assertEqual(result, ".pkl")

def test_compression_suffix_unknown_method(self):
"""Test that .pkl is returned for unknown compression 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": "unknown_method"}, f)

result = compression_suffix(self.mock_context)
self.assertEqual(result, ".pkl")


class TestCompressionOptions(unittest.TestCase):
"""Test cases for compression_options function."""

def setUp(self):
self.temp_dir = tempfile.mkdtemp()
self.mock_context = MagicMock()
self.mock_context.pkg_data_dir = self.temp_dir

def tearDown(self):
import shutil
shutil.rmtree(self.temp_dir, ignore_errors=True)

def test_compression_options_returns_none_when_no_file(self):
"""Test that None is returned when options file doesn't exist."""
from oocana.oocana.serialization import compression_options

result = compression_options(self.mock_context)
self.assertIsNone(result)

def test_compression_options_returns_dict_when_file_exists(self):
"""Test that options are returned when file exists."""
from oocana.oocana.serialization import compression_options, COMPRESSION_OPTIONS_FILE

expected = {"method": "gzip"}
with open(os.path.join(self.temp_dir, COMPRESSION_OPTIONS_FILE), "w") as f:
json.dump(expected, f)

result = compression_options(self.mock_context)
self.assertEqual(result, expected)

def test_compression_options_handles_invalid_json(self):
"""Test that None is returned for invalid JSON."""
from oocana.oocana.serialization import compression_options, COMPRESSION_OPTIONS_FILE

with open(os.path.join(self.temp_dir, COMPRESSION_OPTIONS_FILE), "w") as f:
f.write("not valid json")

result = compression_options(self.mock_context)
self.assertIsNone(result)


if __name__ == '__main__':
unittest.main()
Loading