Skip to content

Add load_policies_from_adapter for database policy loading on startup - #33

Closed
hsluoyz with Copilot wants to merge 8 commits into
masterfrom
copilot/add-auto-load-policies
Closed

Add load_policies_from_adapter for database policy loading on startup#33
hsluoyz with Copilot wants to merge 8 commits into
masterfrom
copilot/add-auto-load-policies

Conversation

Copilot AI commented Oct 30, 2025

Copy link
Copy Markdown
Contributor
  • Analyze the current codebase structure
  • Add load_policies_from_adapter/1 to EnforcerServer module
  • Add handler for load_policies_from_adapter in GenServer callbacks
  • Create comprehensive tests for the new functionality
  • Add integration tests demonstrating the exact use case from the issue
  • Improve documentation for Enforcer.load_policies!/1
  • Add comprehensive database persistence section to README
  • Apply code formatting
  • Address code review feedback - improve error handling in tests
  • Add exception handling to prevent GenServer crashes
  • Fix compilation warning - reorder function definitions to prevent @doc conflicts
  • All CI checks passing

Implementation Complete

Successfully implemented load_policies_from_adapter/1 functionality with all CI checks passing.

Original prompt

This section details on the original issue you should resolve

<issue_title>EctoAdapter: No Built-in Way to Load Policies from Database on Startup</issue_title>
<issue_description>## Summary
The EctoAdapter automatically saves policies to the database but provides no clean way to load them back into the enforcer's memory on application startup. This creates an asymmetric API and forces developers to implement manual workarounds.

Current Behavior

What Works (Auto-Save)

# Configure adapter
adapter = EctoAdapter.new(Repo)
EnforcerServer.set_persist_adapter("my_enforcer", adapter)

# Add policy - automatically saved to DB
EnforcerServer.add_policy("my_enforcer", {:p, ["admin", "data", "write", "org:abc"]})
# ✅ Policy is now in both memory AND database

What Doesn't Work (No Auto-Load)

# Application restart...

# Re-configure adapter
adapter = EctoAdapter.new(Repo)
EnforcerServer.set_persist_adapter("my_enforcer", adapter)

# ❌ Policies from database are NOT loaded into memory
EnforcerServer.allow?("my_enforcer", ["admin", "data", "write", "org:abc"])
# => false (policies exist in DB but not in memory)

Expected API

There should be a clean, built-in way to load policies from the database:
# Option 1: Automatic on adapter setup
adapter = EctoAdapter.new(Repo)
EnforcerServer.set_persist_adapter("my_enforcer", adapter, load: true)

# Option 2: Explicit load function
EnforcerServer.load_policies_from_adapter("my_enforcer")

# Option 3: Enhanced load_policies that works with adapters
EnforcerServer.load_policies("my_enforcer", :from_adapter)

Current Workaround (Manual Implementation)

Developers must implement their own loading logic by querying the database directly and manually adding each policy:

defp load_policies_from_db do
  # Manually query the database
  rules = Repo.all(Acx.Persist.EctoAdapter.CasbinRule)

  # Manually add each rule to the enforcer's memory
  Enum.each(rules, fn rule ->
    case rule.ptype do
      "p" ->
        attrs = build_attrs([rule.v0, rule.v1, rule.v2, rule.v3, rule.v4, rule.v5, rule.v6])
        EnforcerServer.add_policy(@enforcer_name, {:p, attrs})


      "g" ->
        attrs = build_attrs([rule.v0, rule.v1, rule.v2])
        case length(attrs) do
          3 ->
            [child, parent, domain] = attrs
            EnforcerServer.add_mapping_policy(@enforcer_name, {:g, child, parent, domain})
          2 ->
            [child, parent] = attrs
            EnforcerServer.add_mapping_policy(@enforcer_name, {:g, child, parent})
        end
    end
  end)
end

defp build_attrs(values) do
  Enum.reject(values, &is_nil/1)
end

Why This is Problematic

  1. The set_persist_adapter/2 function suggests persistence is fully configured, but it only handles saves, not loads.
  2. New users expect load_policies/2 to work with adapters, but it only accepts file paths:
# This signature is misleading:
EnforcerServer.load_policies(name, file)
# "file" parameter suggests it ONLY loads from files
  1. Comparison with PersistAdapter.load_policies/1
The library DOES have a function that retrieves policies from the adapter:
{:ok, {policies, grouping_policies}} = Acx.Persist.PersistAdapter.load_policies(adapter)

But: There's no function to feed these policies back into the enforcer's memory. Developers must manually loop through and add each one.

Proposed Solutions

Option 1: Add load_policies_from_adapter/1
EnforcerServer.load_policies_from_adapter("my_enforcer")

Option 2: Enhance set_persist_adapter/3 with options
EnforcerServer.set_persist_adapter("my_enforcer", adapter, auto_load: true)

Option 3: Make load_policies/2 adapter-aware
Accept atom :adapter as second parameter
EnforcerServer.load_policies("my_enforcer", :adapter)

Option 4: Add Enforcer.load_policies_from_adapter/2
enforcer = Enforcer.init(model_path)
enforcer = Enforcer.load_policies_from_adapter(enforcer, adapter)</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@hsluoyz
hsluoyz marked this pull request as ready for review October 30, 2025 07:09
Copilot AI and others added 6 commits October 30, 2025 07:19
Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
Copilot AI changed the title [WIP] Add auto-loading of policies from database on startup Add load_policies_from_adapter for database policy loading on startup Oct 30, 2025
Copilot AI requested a review from hsluoyz October 30, 2025 07:38
@hsluoyz

hsluoyz commented Oct 30, 2025

Copy link
Copy Markdown
Member

@copilot fix failed CI check:

Run mix compile --warnings-as-errors
  mix compile --warnings-as-errors
  shell: /usr/bin/bash -e {0}
  env:
    INSTALL_DIR_FOR_OTP: /home/runner/work/_temp/.setup-beam/otp
    INSTALL_DIR_FOR_ELIXIR: /home/runner/work/_temp/.setup-beam/elixir
Compiling 2 files (.ex)
warning: redefining @doc attribute previously set at line 266
Warning:   lib/acx/enforcer.ex:323: Acx.Enforcer (module)

Compilation failed due to warnings while using the --warnings-as-errors option
Error: Process completed with exit code 1.

Move load_policies!/2 before load_policies!/1 so each arity has its own @doc block without conflicts.

Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>

Copilot AI commented Oct 30, 2025

Copy link
Copy Markdown
Contributor Author

@copilot fix failed CI check:

...

Fixed in commit 6d9950b. The warning was caused by having two @doc attributes for different arities of load_policies!. Reordered the function definitions so load_policies!/2 comes before load_policies!/1, ensuring each arity has its own documentation block without conflicts.

@hsluoyz hsluoyz closed this Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EctoAdapter: No Built-in Way to Load Policies from Database on Startup

3 participants