ejabberd icon indicating copy to clipboard operation
ejabberd copied to clipboard

Fix MUC room created_at timestamp and add updated_at field

Open YonesSohrabi opened this issue 5 months ago • 5 comments

Fixes

This PR fixes the following issues:

  • Fixes #3849 - MUC room created_at timestamp incorrectly set to 1970-01-02
  • Fixes #4465 - Need to track MUC room modification time

Summary

This PR fixes the issue where newly created MUC rooms have created_at = 1970-01-02 00:00:00 and adds proper separation between creation time and update time.

Problem

Currently:

  • ❌ New rooms get created_at = 1970-01-02 00:00:00
  • created_at changes on every room update (UPSERT behavior)
  • ❌ No way to track when room configuration was modified
  • ❌ Semantic confusion between hibernation_time and created_at

Solution

This PR:

  1. ✅ Adds updated_at column to muc_room table
  2. ✅ Ensures created_at is set once on creation and never changes
  3. ✅ Tracks room modifications with updated_at
  4. ✅ Separates concerns: created_atupdated_athibernation_time
  5. ✅ Adds hibernation_time support in API (optional)

Changes

Modified Files

src/mod_muc_sql.erl (lines 75-78, 142-185, 295)

  • Added updated_at column to schema
  • Modified store_room/5 to check if room exists:
    • Existing room: Preserve created_at, update updated_at
    • New room: Set both to current time
  • Removed hardcoded 1970-01-02 00:00:00 fallback
  • Fixed get_hibernated_rooms_older_than query

src/mod_muc_admin.erl (line 1734)

  • Added hibernation_time support in format_room_option
  • Allows API users to optionally set hibernation time

New Files

  • PULL_REQUEST_PROPOSAL.md - Detailed analysis with alternatives
  • MIGRATION_GUIDE.md - Step-by-step migration instructions
  • sql/migration_add_updated_at.sql - SQL migration script

Database Migration Required

-- MySQL/MariaDB
ALTER TABLE muc_room ADD COLUMN updated_at timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP;
UPDATE muc_room SET updated_at = created_at;

-- PostgreSQL
ALTER TABLE muc_room ADD COLUMN updated_at timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP;
UPDATE muc_room SET updated_at = created_at;

See MIGRATION_GUIDE.md for complete instructions for all databases.

Testing

Before This PR

# Create room
curl -X POST /api/create_room_with_opts -d '{...}'

# Check database
SELECT name, created_at FROM muc_room WHERE name='testroom';
# Result: created_at = 1970-01-02 00:00:00 ❌

After This PR

# Create room
curl -X POST /api/create_room_with_opts -d '{...}'

# Check database
SELECT name, created_at, updated_at FROM muc_room WHERE name='testroom';
# Result: 
# created_at = 2025-01-07 12:34:56 ✅
# updated_at = 2025-01-07 12:34:56 ✅

# Update room configuration
ejabberdctl change_room_option testroom conference.localhost title "New Title"

# Check database again
SELECT name, created_at, updated_at FROM muc_room WHERE name='testroom';
# Result:
# created_at = 2025-01-07 12:34:56 ✅ (unchanged!)
# updated_at = 2025-01-07 13:45:00 ✅ (updated!)

Backward Compatibility

Fully backward compatible with migration script
No breaking changes to API
Existing rooms work without changes
Old rooms can be updated with migration script

Benefits

  1. Accurate timestamps: Room creation time is correct
  2. Track changes: Know when room configuration was modified
  3. Better analytics: Proper data for reporting
  4. Clear semantics: Separate creation, update, and hibernation concepts
  5. Industry standard: Follows common created_at + updated_at pattern

Documentation

  • PULL_REQUEST_PROPOSAL.md: Detailed problem analysis and alternative solutions
  • MIGRATION_GUIDE.md: Complete migration instructions with troubleshooting
  • sql/migration_add_updated_at.sql: Ready-to-use SQL migration scripts

Checklist

  • [x] Code follows ejabberd style guidelines
  • [x] Changes are backward compatible
  • [x] Migration guide included
  • [x] SQL migration scripts included
  • [x] Documentation updated
  • [x] No breaking changes to API
  • [x] Tested manually (see Testing section)

Related Files

  • src/mod_muc_sql.erl - Main changes
  • src/mod_muc_admin.erl - API support
  • PULL_REQUEST_PROPOSAL.md - Detailed proposal
  • MIGRATION_GUIDE.md - Migration instructions

YonesSohrabi avatar Oct 07 '25 13:10 YonesSohrabi

Hi @neustradamus and @prefiks ,

Thank you for the feedback! I've updated the PR based on the review comments:

Changes Made

1. Simplified store_room/5 using UPSERT special syntax

  • Replaced conditional logic with -created_at syntax
  • This uses ejabberd's built-in feature where -field means "only set on INSERT, not on UPDATE"
  • Reduced code from ~30 lines to ~10 lines
  • Improved performance (one less query)

**Before:ang case ejabberd_sql:sql_query_t(...) of {selected, [{CreatedAt}]} -> ?SQL_UPSERT_T("muc_room", [..., "created_at=%(CreatedAt)t", ...]); _ -> ?SQL_UPSERT_T("muc_room", [..., "created_at=%(CurrentTime)t", ...]) endAfter:**ang ?SQL_UPSERT_T( "muc_room", ["!name=%(Name)s", "!host=%(Host)s", "server_host=%(LServer)s", "opts=%(SOpts)s", "-created_at=%(CurrentTime)t", % Only set on INSERT "updated_at=%(CurrentTime)t"]) % Set on both INSERT and UPDATE### 2. Updated SQL schema files

  • sql/mysql.new.sql - Added updated_at column
  • sql/pg.new.sql - Added updated_at column
  • sql/lite.new.sql - Added updated_at column
  • sql/mssql.new.sql - Added updated_at column

Related Issues

This PR fixes:

  • #3849 - MUC room created_at timestamp issue
  • #4465 - Need to track room modification time

Testing

Tested manually:

  1. ✅ New room creation: created_at and updated_at set correctly
  2. ✅ Room update: created_at preserved, updated_at updated
  3. ✅ No breaking changes to API
  4. ✅ Migration script works on existing databases

Please let me know if there are any other changes needed!

YonesSohrabi avatar Nov 15 '25 07:11 YonesSohrabi

@neustradamus Thank you for the feedback!

I've updated the PR:

Updated all SQL files:

  • sql/mysql.sql
  • sql/mysql.new.sql
  • sql/pg.sql
  • sql/pg.new.sql
  • sql/lite.sql
  • sql/lite.new.sql
  • sql/mssql.sql
  • sql/mssql.new.sql

Added issue references:

  • #3849 - MUC room created_at timestamp issue
  • #4465 - Track room modification time

Simplified code using -created_at UPSERT syntax

All SQL schema files now include the updated_at column. The PR is ready for review!

YonesSohrabi avatar Nov 15 '25 09:11 YonesSohrabi

Coverage Status

coverage: 33.788% (+0.005%) from 33.783% when pulling ec12913ad6117ea5a811c2abe47410f3a8fb2f7d on YonesSohrabi:fix-muc-room-timestamps into 71dccedfc64dd40a5cd0e98bb1ecece07ea379b9 on processone:master.

coveralls avatar Nov 17 '25 19:11 coveralls

I will try to review this today

prefiks avatar Nov 18 '25 08:11 prefiks

Hi @YonesSohrabi, many thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

p1bot avatar Nov 18 '25 16:11 p1bot