graham/towerops
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions
towerops/REFACTOR.md
Graham McIntire 22ae257b60 refactor
2026-01-31 09:35:07 -06:00

11 KiB
Raw Blame History

Custom Ecto Types Refactor - Progress Tracker

Overview

Implementing custom Ecto types to avoid "primitive obsession" by creating rich domain objects for IP addresses, email addresses, and MAC addresses. This follows the pattern from https://www.elixirstreams.com/tips/richer-domain-types-with-ecto-custom-types.

Current Phase: Phase 1 - IP Address Custom Type Status: In Progress (1 of 7 schemas migrated) Started: 2026-01-31

Phase 1: IP Address Custom Type

Completed ✅

  1. IPAddress Custom Type Implementation (lib/towerops/ecto_types/ip_address.ex)

    • Implements Ecto.Type behavior with all 4 callbacks
    • Validates IPv4 and IPv6 addresses using :inet.parse_address/1
    • Provides helpers: new/1, new!/1, ipv4?/1, ipv6?/1, to_string/1, to_tuple/1
    • Handles edge cases:
      • nil values
      • IPv6 zone IDs (strips before validation)
      • CIDR notation (rejects)
      • BSD-style IP notation (accepts, matches Erlang behavior)
    • JSON Encoding: Added @derive {Jason.Encoder, only: [:address]} for GDPR data export compatibility
    • Database type: :string (no migrations needed)

  2. Comprehensive Unit Tests (test/towerops/ecto_types/ip_address_test.exs)

    • 44 tests covering all callbacks and edge cases
    • Tests for: type/0, cast/1, load/1, dump/1, roundtrips, helpers
    • 100% coverage of custom type module
    • All tests passing ✅

  3. Schema Migration #1: Towerops.Admin.AuditLog

    • File: lib/towerops/admin/audit_log.ex
    • Changed: field :ip_address, :string → field :ip_address, Towerops.EctoTypes.IpAddress
    • Risk Level: Lowest (append-only audit logs)
    • No validation code to remove (didn't have any)
    • Tests: All account data controller tests passing ✅
    • JSON serialization verified working for GDPR data export

  4. Documentation Updated

    • Added "Custom Ecto Types" section to CLAUDE.md (after "Development Environment")
    • Includes: overview, available types, usage examples, implementation pattern, testing pattern
    • Plan file created at /Users/graham/.claude/plans/swift-riding-mountain.md

In Progress 🔄

Current Task: Document remaining work before continuing

Remaining Schemas to Migrate (6 schemas)

Low Risk (migrate next)

  1. Towerops.Accounts.BrowserSession (lib/towerops/accounts/browser_session.ex)

    • Change: field :ip_address, :string → field :ip_address, Towerops.EctoTypes.IpAddress
    • Risk: Low (session tracking)
    • Notes: Keep validate_length(:ip_address, max: 45) if present (optional, harmless)
    • Tests to update: test/towerops/accounts/browser_session_test.exs (if exists)

  2. Towerops.Accounts.LoginAttempt (lib/towerops/accounts/login_attempt.ex)

    • Change: field :ip_address, :string → field :ip_address, Towerops.EctoTypes.IpAddress
    • Risk: Low (logging only)
    • Tests to update: Check for related tests

Medium Risk (migrate after low risk)

  1. Towerops.Snmp.ArpEntry (lib/towerops/snmp/schemas/arp_entry.ex)

    • Change: field :ip_address, :string → field :ip_address, Towerops.EctoTypes.IpAddress
    • Risk: Medium (SNMP discovery populates this)
    • Testing: Run SNMP discovery on test device after migration
    • Tests to update: test/towerops/snmp/*_test.exs

  2. Towerops.Snmp.Neighbor (lib/towerops/snmp/schemas/neighbor.ex)

    • Change: field :remote_address, :string → field :remote_address, Towerops.EctoTypes.IpAddress
    • Risk: Medium (optional field, may be nil)
    • Note: Field name is remote_address, not ip_address
    • Tests to update: SNMP neighbor discovery tests

High Risk (migrate last)

  1. Towerops.Devices.Device (lib/towerops/devices/device.ex) ⚠️ HIGH RISK

    • Change: field :ip_address, :string → field :ip_address, Towerops.EctoTypes.IpAddress
    • Remove validation code (lines 121-141 in current version):
      • validate_ip_address/1 function
      • parse_ip/1 function
      • Call to |> validate_ip_address() in changeset
    • Risk: High (core Device schema, heavily used)
    • Tests to update extensively: test/towerops/devices_test.exs
      • Add assertions: assert %Towerops.EctoTypes.IpAddress{} = device.ip_address
      • Verify: assert device.ip_address.version == :ipv4
      • Test invalid IP still produces changeset error
      • Test IPv6 addresses (lines 95-99 already exist)

  2. Towerops.Snmp.IpAddress (lib/towerops/snmp/schemas/ip_address.ex) ⚠️ COMPLEX

    • Change: field :ip_address, :string → field :ip_address, Towerops.EctoTypes.IpAddress
    • Keep: validate_prefix_length/1 (validates against ip_type field)
    • Add: Helper to auto-populate ip_type from IPAddress.version
    • Related fields: ip_type, prefix_length, subnet_mask
    • Future consideration: Could derive ip_type from IPAddress.version and remove redundant field
    • Risk: Complex (multiple related fields, SNMP-specific)

Testing Strategy

Per-Schema Testing Checklist

For each schema migration:

  • Update schema field type
  • Remove manual IP validation (if exists)
  • Update related tests to assert on %IpAddress{} struct
  • Run schema-specific tests: mix test test/path/to/schema_test.exs
  • Run related integration tests
  • Commit with message: "Migrate [Schema] to IPAddress custom type"

Final Testing (after all 7 schemas)

  • Run full test suite: mix test
  • Check coverage: mix test --cover (target >90%)
  • Run precommit checks: mix precommit
  • Run Dialyzer: mix dialyzer
  • Manual testing:
    • Device CRUD operations
    • SNMP discovery on test device
    • Audit log creation
    • GDPR data export

Known Issues / Edge Cases Handled

  1. JSON Encoding: Fixed by adding @derive {Jason.Encoder, only: [:address]} to IPAddress struct

    • Required for GDPR data export API (/api/v1/account/data)
    • Serializes as just the IP address string, not the full struct

  2. BSD-Style IP Notation: :inet.parse_address/1 accepts "192.168.1" as valid (interprets as "192.168.0.1")

    • This is expected Erlang behavior
    • Updated tests to document this, not reject it

  3. Database Compatibility: No migrations needed

    • Column type stays as varchar(45)
    • Existing data valid on first load
    • Ecto handles conversion transparently via load/1 callback

  4. IPv6 Zone IDs: Stripped before validation (fe80::1%eth0 → fe80::1)

    • Zone IDs are interface-specific and not stored

  5. CIDR Notation: Rejected by custom type

    • Use separate prefix_length field instead
    • Prevents confusion between host addresses and network addresses

Performance Considerations

  • Parsing Overhead: ~1-5 microseconds per IP address (negligible)
  • Memory: ~40 bytes per struct vs ~15-45 for string (~25 byte increase)
  • Query Performance: Unchanged (database still stores strings)
  • JSON Serialization: Efficient (only address field encoded)

Rollback Plan

If issues arise after deployment:

  1. Revert schema field changes to :string (git revert)
  2. Restore validate_ip_address/1 functions (git revert)
  3. No database rollback needed (data unchanged, still stored as strings)
  4. Redeploy previous version

Future Phases (Not Started)

Phase 2: Email Address Custom Type

  • Impact: 4 schemas affected
  • Risk: Medium (user-facing data)
  • Complexity: Medium (normalization + case-insensitive handling)
  • Features:
    • Normalize to lowercase
    • Split into local + domain parts
    • Consistent validation across schemas
  • Schemas:
    • Towerops.Accounts.User (email field)
    • Others TBD (need to search codebase)

Phase 3: MAC Address Custom Type

  • Impact: 3 schemas affected
  • Risk: Low (SNMP-specific)
  • Complexity: Medium (multiple format handling)
  • Features:
    • Handle multiple input formats (colon, hyphen, dot-separated)
    • Normalize to colon-separated lowercase
    • SNMP binary conversion support
  • Schemas: TBD (need to search SNMP schemas)

Phase 4: Optional Enhancements

  • Derive ip_type from IPAddress.version in Towerops.Snmp.IpAddress
  • Create SubnetMask custom type
  • Create IPNetwork custom type (CIDR support)

Commit History

  1. feat: add IPAddress custom Ecto type with comprehensive tests

    • Created lib/towerops/ecto_types/ip_address.ex
    • Created test/towerops/ecto_types/ip_address_test.exs
    • Added Jason.Encoder derivation for JSON compatibility
    • All 44 tests passing

  2. docs: add Custom Ecto Types section to CLAUDE.md

    • Added comprehensive documentation after "Development Environment" section
    • Includes pattern, usage examples, testing guidelines

  3. refactor: migrate AuditLog schema to IPAddress custom type

    • Changed field :ip_address, :string → field :ip_address, Towerops.EctoTypes.IpAddress
    • No validation changes needed (schema didn't have IP validation)
    • All account data controller tests passing

Next Steps

When resuming this work:

  1. Continue with low-risk schemas (BrowserSession, LoginAttempt)
  2. Move to medium-risk SNMP schemas (ArpEntry, Neighbor)
  3. Carefully migrate high-risk Device schema with extensive testing
  4. Complete complex IpAddress schema migration
  5. Run full test suite and coverage check
  6. Deploy to staging and monitor
  7. Deploy to production

Estimated Time Remaining: 4-8 hours for Phase 1 completion

Reference Files

Pattern to Follow:

  • lib/towerops/ecto_types/json_any.ex - Existing custom type
  • test/towerops/ecto_types/json_any_test.exs - Test pattern (184 lines)

Modified Files (so far):

  • lib/towerops/ecto_types/ip_address.ex (NEW)
  • test/towerops/ecto_types/ip_address_test.exs (NEW)
  • lib/towerops/admin/audit_log.ex (MODIFIED)
  • CLAUDE.md (MODIFIED - added Custom Ecto Types section)

Files to Modify (remaining):

  • lib/towerops/accounts/browser_session.ex
  • lib/towerops/accounts/login_attempt.ex
  • lib/towerops/snmp/schemas/arp_entry.ex
  • lib/towerops/snmp/schemas/neighbor.ex
  • lib/towerops/devices/device.ex (HIGH RISK)
  • lib/towerops/snmp/schemas/ip_address.ex (COMPLEX)

Success Criteria for Phase 1 Completion

  • IPAddress custom type implemented with all 4 callbacks
  • >90% test coverage on custom type (100% achieved)
  • All 7 schemas migrated successfully (1/7 complete)
  • Duplicate validation code removed from changesets
  • All existing tests pass
  • No database migrations required (verified ✅)
  • Documentation updated in CLAUDE.md
  • Code reviewed and deployed to production

Progress: 30% complete (foundation laid, 1 schema migrated, 6 remaining)