# SVC-006 Migration Plan: Move db.commit() from Services to Endpoints

## Overview

**Objective:** Migrate all `db.commit()` calls from service layer to API endpoints to follow the industry-standard transaction control pattern.

**Rule:** SVC-006 - Services must NOT call `db.commit()`

**Principle:** One request = one transaction, with commit controlled at the API endpoint level.

---

## Benefits

| Benefit | Description |
|---------|-------------|
| **Composability** | Multiple service calls can be composed in a single transaction |
| **Clean rollback** | If any operation fails, the entire request rolls back automatically |
| **Testability** | Services can be tested in isolation without actual commits |
| **Consistency** | All transactions follow the same pattern |

---

## Migration Pattern

### Before (Anti-pattern)
```python
# Service
def create_store(self, db: Session, data: StoreCreate) -> Store:
    store = Store(**data.model_dump())
    db.add(store)
    db.commit()      # ❌ Service commits
    db.refresh(store)
    return store

# Endpoint
def create_store_endpoint(...):
    store = store_service.create_store(db, data)
    return StoreResponse.model_validate(store)
```

### After (Correct pattern)
```python
# Service
def create_store(self, db: Session, data: StoreCreate) -> Store:
    store = Store(**data.model_dump())
    db.add(store)
    db.flush()       # ✅ Get ID without committing
    return store

# Endpoint
def create_store_endpoint(...):
    store = store_service.create_store(db, data)
    db.commit()      # ✅ ARCH: Commit at API level for transaction control
    return StoreResponse.model_validate(store)
```

### Key Changes

1. **Replace `db.commit()` with `db.flush()`** in services when you need the ID
2. **Remove `db.refresh()`** from services (endpoint can do this if needed)
3. **Remove `db.rollback()`** from service exception handlers (endpoint handles this)
4. **Add `db.commit()`** to the endpoint after service call(s)

---

## Services to Migrate

### Priority 1: Core Business Services (High Impact)
| Service | Commits | Complexity | Endpoints to Update |
|---------|---------|------------|---------------------|
| `admin_service.py` | 6 | Medium | Multiple admin endpoints |
| `inventory_service.py` | 9 | High | Inventory management endpoints |
| `product_service.py` | 3 | Medium | Product CRUD endpoints |
| `order_service.py` | 2 | Medium | Order processing endpoints |

### Priority 2: Domain Services (Medium Impact)
| Service | Commits | Complexity | Endpoints to Update |
|---------|---------|------------|---------------------|
| `store_domain_service.py` | 4 | Medium | Domain management endpoints |
| `store_team_service.py` | 5 | Medium | Team management endpoints |
| `store_theme_service.py` | 3 | Low | Theme endpoints |
| `customer_service.py` | 4 | Medium | Customer endpoints |
| `cart_service.py` | 5 | Medium | Cart/checkout endpoints |

### Priority 3: Supporting Services (Lower Impact)
| Service | Commits | Complexity | Endpoints to Update |
|---------|---------|------------|---------------------|
| `content_page_service.py` | 3 | Low | CMS endpoints |
| `marketplace_product_service.py` | 3 | Low | Marketplace endpoints |
| `team_service.py` | 2 | Low | Team endpoints |
| `admin_settings_service.py` | 3 | Low | Settings endpoints |
| `code_quality_service.py` | 5 | Low | Internal tooling |

### Priority 4: Auth & Audit (Special Handling)
| Service | Commits | Notes |
|---------|---------|-------|
| `auth_service.py` | 1 | Handle carefully - auth flows |
| `admin_audit_service.py` | 1 | May need immediate commit for audit trail |
| `marketplace_import_job_service.py` | 1 | Background job - special handling |

### Excluded
| Service | Commits | Reason |
|---------|---------|--------|
| `log_service.py` | 2 | Audit logs require immediate commits |

---

## Completed Migrations

- [x] `store_service.py` (6 commits → 0) - Commit: 6bd3af0

---

## Migration Steps (Per Service)

### Step 1: Identify Commits
```bash
grep -n "db.commit()" app/services/<service_name>.py
```

### Step 2: Find Calling Endpoints
```bash
grep -rn "<service>_service\.<method>" app/api/ --include="*.py"
```

### Step 3: Update Service
For each method with `db.commit()`:

1. **If creating entities:**
   ```python
   # Before
   db.add(entity)
   db.commit()
   db.refresh(entity)

   # After
   db.add(entity)
   db.flush()  # Get ID without committing
   ```

2. **If updating entities:**
   ```python
   # Before
   entity.field = value
   db.commit()
   db.refresh(entity)

   # After
   entity.field = value
   # No commit - endpoint handles it
   ```

3. **Remove rollback from exception handlers:**
   ```python
   # Before
   except SomeException:
       db.rollback()
       raise

   # After
   except SomeException:
       raise  # Endpoint handles rollback
   ```

### Step 4: Update Endpoints
Add `db.commit()` after service calls:
```python
result = some_service.some_method(db, data)
db.commit()  # ✅ ARCH: Commit at API level for transaction control
return SomeResponse(...)
```

### Step 5: Validate
```bash
# Check no commits remain in service
grep -n "db.commit()" app/services/<service_name>.py

# Run architecture validator
python scripts/validate/validate_architecture.py -o <entity_name>

# Run tests
pytest tests/test_<service_name>.py -v
```

### Step 6: Commit
```bash
git add app/services/<service>.py app/api/v1/...
git commit -m "refactor(<entity>): migrate db.commit() from service to endpoints (SVC-006)"
```

---

## Testing Strategy

### Unit Tests
- Services should be tested with `db.flush()` (no actual commit)
- Mock the session or use a test transaction that rolls back

### Integration Tests
- Test full request flow including commit
- Verify data persists after endpoint call
- Verify rollback on exception

### Manual Testing
After each migration:
1. Test create operations - verify data persists
2. Test update operations - verify changes persist
3. Test error scenarios - verify no partial commits

---

## Rollback Strategy

If issues are found after migration:

1. **Revert the commit:**
   ```bash
   git revert <commit_hash>
   ```

2. **Or fix forward:**
   - Add `db.commit()` back to service temporarily
   - Investigate the endpoint that's missing commit
   - Fix and re-deploy

---

## Progress Tracking

Run this command to check remaining violations:
```bash
python scripts/validate/validate_architecture.py 2>&1 | grep "SVC-006" | wc -l
```

Current status: **60 remaining** (down from 66)

---

## Timeline Recommendation

This migration can be done gradually:

1. **Immediate:** Fix as you touch each service for other changes
2. **Batch:** Dedicate time to migrate 2-3 services per session
3. **Priority:** Focus on Priority 1 services first for maximum impact

The SVC-006 rule is set to **warning** (not error), so CI won't fail. This allows gradual migration without blocking deployments.

---

## Commands Reference

```bash
# Count remaining violations
python scripts/validate/validate_architecture.py 2>&1 | grep "SVC-006" | wc -l

# List services with commits (sorted by count)
grep -c "db.commit()" app/services/*.py | grep -v ":0$" | sort -t: -k2 -n

# Validate specific entity
python scripts/validate/validate_architecture.py -o store

# Validate specific file
python scripts/validate/validate_architecture.py -f app/services/admin_service.py
```