Skip to content

Commit 403e121

Browse files
committed
Refactor and improve code quality
Refactor and improve code quality This commit includes several code improvements and refactorings: 1. **Refactored `app/core/models.py` for log counting:** * Introduced a private helper method `_count_logs_since` in the `SystemLog` model to consolidate logic for counting logs based on time, level, and security status. * Updated `get_error_count_last_24h` and `get_security_events_count_last_24h` to use this new helper method, reducing code duplication. 2. **Made security log retention configurable:** * Added a `SECURITY_LOG_RETENTION_DAYS` configuration variable in `app/config.py` (defaulting to 90 days, settable via environment variable). * Modified the `cleanup_old_logs` method in `SystemLog` to use this configuration value instead of a hardcoded one. 3. **Improved `app/core/models.py` documentation and readability:** * Added a docstring to `LogSearchQuery.to_dict()`. * Ensured dictionary definitions and SQLAlchemy queries in `SystemLog` methods are formatted for better readability. * Removed some redundant local imports of `datetime` and `timedelta`. 4. **Refactored exception handling in `app/auth/routes.py`:** * Introduced a private helper function `_handle_auth_exception` to standardize logging and flash messages for exceptions occurring during user login and registration. * Updated the `login` and `register` routes to use this helper, reducing boilerplate. These changes aim to improve code maintainability, readability, and configurability.
1 parent 8a97e13 commit 403e121

3 files changed

Lines changed: 37 additions & 31 deletions

File tree

app/auth/routes.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@
99

1010
auth_bp = Blueprint('auth', __name__, url_prefix='/auth')
1111

12+
def _handle_auth_exception(e, operation_name):
13+
"""Helper function to log and flash authentication errors."""
14+
current_app.logger.error(f"{operation_name} error: {str(e)}")
15+
current_app.logger.error(traceback.format_exc())
16+
flash(f'An error occurred during {operation_name.lower()}. Please try again.', 'danger')
17+
1218
@auth_bp.route('/login', methods=['GET', 'POST'])
1319
def login():
1420
if current_user.is_authenticated:
@@ -29,9 +35,7 @@ def login():
2935
next_page = url_for('core.index')
3036
return redirect(next_page)
3137
except Exception as e:
32-
current_app.logger.error(f"Login error: {str(e)}")
33-
current_app.logger.error(traceback.format_exc())
34-
flash('An error occurred during login. Please try again.', 'danger')
38+
_handle_auth_exception(e, "Login")
3539
return render_template('auth/login.html', title='Sign In', form=form)
3640

3741
@auth_bp.route('/logout')
@@ -59,10 +63,8 @@ def register():
5963
flash('Congratulations, you are now a registered user!', 'success')
6064
return redirect(url_for('auth.login'))
6165
except Exception as e:
62-
db.session.rollback()
63-
current_app.logger.error(f"Registration error: {str(e)}")
64-
current_app.logger.error(traceback.format_exc())
65-
flash('An error occurred during registration. Please try again.', 'danger')
66+
db.session.rollback() # Keep rollback here as it's specific to DB operations
67+
_handle_auth_exception(e, "Registration")
6668
return render_template('auth/register.html', title='Register', form=form)
6769

6870
@auth_bp.route('/profile')

app/config.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ class Config:
2121
# Modules repository URL
2222
MODULES_REPOSITORY_URL = os.environ.get('MODULES_REPOSITORY_URL') or \
2323
'https://github.com/CoreSecFrame/CoreSecFrame-Modules'
24+
25+
# Log retention settings
26+
SECURITY_LOG_RETENTION_DAYS = int(os.environ.get('SECURITY_LOG_RETENTION_DAYS', 90))
2427

2528
# Security settings
2629
SESSION_COOKIE_SECURE = False # Set to True in production

app/core/models.py

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# app/core/models.py
2+
from flask import current_app
23
from app import db
3-
from datetime import datetime
4+
from datetime import datetime, timedelta
45
from sqlalchemy import Index
56

67
class SystemLog(db.Model):
@@ -102,6 +103,20 @@ def is_error(self):
102103
def is_warning(self):
103104
"""Check if this is a warning log"""
104105
return self.level == 'WARNING'
106+
107+
@classmethod
108+
def _count_logs_since(cls, since_hours, levels=None, is_security_event=None):
109+
"""Helper method to count logs within a time window with optional filters."""
110+
since = datetime.utcnow() - timedelta(hours=since_hours)
111+
query = cls.query.filter(cls.timestamp >= since)
112+
113+
if levels:
114+
query = query.filter(cls.level.in_(levels))
115+
116+
if is_security_event is not None:
117+
query = query.filter(cls.is_security_event == is_security_event)
118+
119+
return query.count()
105120

106121
@classmethod
107122
def get_recent_logs(cls, limit=50, level=None, is_security=None, user_id=None):
@@ -122,31 +137,16 @@ def get_recent_logs(cls, limit=50, level=None, is_security=None, user_id=None):
122137
@classmethod
123138
def get_error_count_last_24h(cls):
124139
"""Get count of errors in the last 24 hours"""
125-
from datetime import datetime, timedelta
126-
127-
since = datetime.utcnow() - timedelta(hours=24)
128-
129-
return cls.query.filter(
130-
cls.timestamp >= since,
131-
cls.level.in_(['ERROR', 'CRITICAL'])
132-
).count()
140+
return cls._count_logs_since(since_hours=24, levels=['ERROR', 'CRITICAL'])
133141

134142
@classmethod
135143
def get_security_events_count_last_24h(cls):
136144
"""Get count of security events in the last 24 hours"""
137-
from datetime import datetime, timedelta
138-
139-
since = datetime.utcnow() - timedelta(hours=24)
140-
141-
return cls.query.filter(
142-
cls.timestamp >= since,
143-
cls.is_security_event == True
144-
).count()
145+
return cls._count_logs_since(since_hours=24, is_security_event=True)
145146

146147
@classmethod
147148
def get_log_level_stats(cls, hours=24):
148149
"""Get statistics by log level for the specified time period"""
149-
from datetime import datetime, timedelta
150150
from sqlalchemy import func
151151

152152
since = datetime.utcnow() - timedelta(hours=hours)
@@ -163,7 +163,6 @@ def get_log_level_stats(cls, hours=24):
163163
@classmethod
164164
def get_module_stats(cls, hours=24, limit=10):
165165
"""Get top modules by log count"""
166-
from datetime import datetime, timedelta
167166
from sqlalchemy import func
168167

169168
since = datetime.utcnow() - timedelta(hours=hours)
@@ -182,12 +181,11 @@ def get_module_stats(cls, hours=24, limit=10):
182181
@classmethod
183182
def cleanup_old_logs(cls, days_to_keep=30):
184183
"""Clean up old log entries"""
185-
from datetime import datetime, timedelta
186-
187184
cutoff_date = datetime.utcnow() - timedelta(days=days_to_keep)
188185

189-
# Keep security events longer (90 days)
190-
security_cutoff = datetime.utcnow() - timedelta(days=90)
186+
# Keep security events longer, based on config
187+
security_retention_days = current_app.config['SECURITY_LOG_RETENTION_DAYS']
188+
security_cutoff = datetime.utcnow() - timedelta(days=security_retention_days)
191189

192190
# Delete old non-security logs
193191
deleted_count = cls.query.filter(
@@ -227,7 +225,10 @@ class LogSearchQuery(db.Model):
227225
use_count = db.Column(db.Integer, default=0)
228226

229227
def to_dict(self):
230-
"""Convert to dictionary for API"""
228+
"""
229+
Returns a dictionary representation of the log search query,
230+
suitable for API responses.
231+
"""
231232
return {
232233
'id': self.id,
233234
'name': self.name,

0 commit comments

Comments
 (0)