Fix PlugConfig cache miss for empty list

[vipulpandey21]

Proposed Changes

• Fixed a race condition where the PlugConfig list cache was cleared before the database write, allowing concurrent requests to repopulate the cache with stale data.
• Updated the cache invalidation order so the database is always written before clearing the cache.
• Added missing docstrings to improve code documentation and satisfy quality checks.

---

Associated Issue

• This issue was discovered during local backend validation while working on caching behavior. There was no existing GitHub issue at the time of discovery.

---

Architecture Changes

• No architecture changes.

---

Technical Details

Previous behavior:

• Cache was deleted before serializer.save().
• A concurrent list request could read the old database state and re-cache stale values.

Fix:

serializer.save()
cache.delete(self.cache_key)

This guarantees that the cache is invalidated only after the database is consistent.

---

Verification

python manage.py test
curl http://127.0.0.1:8000/api/v1/plug_config/

Verified that the list endpoint populates the cache correctly and that the cache is invalidated after create, update, and delete operations.

---

Merge Checklist

• Fixed race condition in cache invalidation
• Local tests passing
• Docstrings added
• No breaking API changes

Summary by CodeRabbit

• Bug Fixes

  • Improved caching behavior for plug configuration queries so empty results are correctly recognized and unnecessary refreshes are reduced.

• New Features

  • Read-only access to plug configurations (listing and retrieval) is available without authentication; create/update/delete remain restricted to admin users.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactored PlugConfigViewset: tightened cache miss check to explicit None, moved cache invalidation to after successful create/update/destroy operations, simplified class base-line formatting, and changed permissions so list/retrieve require no permissions while other actions require admin.

Changes

Cohort / File(s)	Summary
Viewset declaration & formatting care/users/api/viewsets/plug_config.py	Collapsed multi-line base-class declaration to a single-line: class PlugConfigViewset(ModelViewSet, GenericViewSet):.
Cache handling care/users/api/viewsets/plug_config.py	list(): treat cache miss as None (if response is None) so falsy cached values are preserved. Moved cache deletion to after serializer.save() / instance.delete() in perform_create, perform_update, and perform_destroy. Removed an inline comment.
Permissions logic care/users/api/viewsets/plug_config.py	get_permissions() now returns [] for list/retrieve actions and [IsAdminUser()] for all other actions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main fix: addressing a cache miss condition for empty PlugConfig lists. It directly corresponds to the primary change of updating the cache validation logic.
Description check	✅ Passed	The description covers all required template sections: Proposed Changes, Associated Issue, and Merge Checklist. It provides detailed technical context, code examples, and verification steps—though specific test coverage and lint status could be slightly more explicit.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[vigneshhari]

Remove all the comments and keep just the changes requested.

[vipulpandey21]

Hi @vigneshhari, I’ve removed all comments and kept only the cache invalidation fix as requested.
Please review when convenient. Thanks!

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

care/users/api/viewsets/plug_config.py (1)

10-10: Redundant base class inheritance.

ModelViewSet already inherits from GenericViewSet, so explicitly listing both is redundant. Consider simplifying to just ModelViewSet.

🔎 Proposed simplification

-class PlugConfigViewset(ModelViewSet, GenericViewSet):
+class PlugConfigViewset(ModelViewSet):

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4592b78 and 96eba77.

📒 Files selected for processing (1)

• care/users/api/viewsets/plug_config.py

🧰 Additional context used

📓 Path-based instructions (1)

**/*.py

📄 CodeRabbit inference engine (.cursorrules)

**/*.py: Prioritize readability and maintainability; follow Django's coding style guide (PEP 8 compliance).
Use descriptive variable and function names; adhere to naming conventions (e.g., lowercase with underscores for functions and variables).

Files:

• care/users/api/viewsets/plug_config.py

🧠 Learnings (2)

📚 Learning: 2024-11-25T11:38:30.225Z

Learnt from: rithviknishad
Repo: ohcnetwork/care PR: 2610
File: plug_config.py:18-23
Timestamp: 2024-11-25T11:38:30.225Z
Learning: In the `care` project, when camera-related code is moved from the main backend to be handled by a plugin, adding a camera plugin configuration (e.g., in `plug_config.py`) is expected, even if the PR title mentions removing camera code.

Applied to files:

• care/users/api/viewsets/plug_config.py

📚 Learning: 2024-12-02T09:14:53.161Z

Learnt from: DraKen0009
Repo: ohcnetwork/care PR: 2585
File: care/users/api/viewsets/user_flag.py:16-30
Timestamp: 2024-12-02T09:14:53.161Z
Learning: In the `UserFlagViewSet` class in `care/users/api/viewsets/user_flag.py`, we don't need to add configurations for `ordering_fields`, `pagination_class`, or `throttle_classes` because default values are already set globally in our Django REST Framework settings.

Applied to files:

• care/users/api/viewsets/plug_config.py

🧬 Code graph analysis (1)

care/users/api/viewsets/plug_config.py (3)

care/users/api/serializers/plug_config.py (1)

• PlugConfigSerializer (6-9)

care/users/models.py (2)

• PlugConfig (215-220)
• get_queryset (31-33)

care/utils/models/base.py (1)

• delete (30-32)

🪛 Ruff (0.14.10)

care/users/api/viewsets/plug_config.py

16-16: Unused method argument: request

(ARG002)

---

16-16: Unused method argument: args

(ARG002)

---

16-16: Unused method argument: kwargs

(ARG002)

🔇 Additional comments (5)

care/users/api/viewsets/plug_config.py (5)

18-18: Good fix for empty list cache handling!

The explicit is None check correctly distinguishes between a cache miss and a cached empty list. The previous falsy check if not response: would incorrectly repopulate the cache when an empty list was the legitimate cached value.

---

24-26: Correct fix for the cache invalidation race condition.

Moving cache invalidation after serializer.save() ensures the database is updated before the cache is cleared, preventing concurrent requests from populating the cache with stale data. The implicit exception handling is also correct—if save() fails, the cache remains valid.

---

28-30: Consistent race condition fix.

Same correct pattern as perform_create—database write happens before cache invalidation, preventing stale cache population.

---

32-34: Race condition fix applied to destroy operation.

Cache invalidation after the delete operation follows the same correct pattern, ensuring the database state is updated before the cache is cleared.

---

36-39: Verify security implications of exposing plugin configurations without authentication.

The list and retrieve actions now require no permissions, allowing unauthenticated access to all plugin configurations. Given that the meta JSONField is flexible and plugin configs often contain credentials or sensitive settings (API keys, endpoints, etc.), this warrants clarification.

Please confirm:

• Is exposing plugin configuration metadata intentionally public?
• If so, was this documented in the PR objectives or commit messages?
• If sensitive data should be protected, these endpoints should at least require authentication.