Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle {:array, $type} types in Migrator #9

Closed

Conversation

ACBullen
Copy link

Previously, a ecto type like {:array, :string} or {:array, :map} would be fed into Code.ensure_loaded?/1, raising an argument error. While it would be nice to be able to handle array types with encoded members, ecto does not seem to handle the encoding of arrays of binaries to postgres nicely, and having the database report the field as an array of binaries leaks more information than just having a binary type for the field and using Cloak.Ecto.Map to wrap the data in a way that can be turned back into the list you want by the app.

Previously, a ecto type like {:array, :string} or {:array, :map} would
be fed into Code.ensure_loaded?/1, raising an argument error. While
it would be nice to be able to handle array types with encoded members,
ecto does not seem to handle the encoding to postgres nicely, and having
postgres report the field as an array of binaries leaks more information
than just having a binary type for the field and using Cloak.Ecto.Map to
wrap the data in a way that can be turned back into the list you want by
the app.
Ignores array types as far as Migrator is concerned
@danielberkompas
Copy link
Owner

Would you mind sharing a copy of the error that you got when trying to use {:array, My.Encrypted.Field} in your schemas?

While I agree that using the {:array, :binary} fields might leak more information than necessary, I think it could still be supported. Couldn't we also fix this by simply protecting the Code.ensure_loaded/1 call from {:array, Ecto.Type} calls?

@ACBullen
Copy link
Author

@danielberkompas Sorry for the delay. I went back and retried the other way with just protecting the Code.ensure_loaded?/1 call and it turned out to work correctly, I'd just had some postgrex issues I needed to fix. I will adjust the fork to do that instead and update the PR

@ACBullen
Copy link
Author

@danielberkompas Pushed up the revised version with an additional test to cover arrays of encrypted values. Feel free to squash it down on merge, since it still has the intermediary state and merge commits in there.

@ACBullen ACBullen changed the title Ignore {:array, $type} types in Migrator Handle {:array, $type} types in Migrator Nov 30, 2020
@danielberkompas
Copy link
Owner

Fixed in #38

1 similar comment
@danielberkompas
Copy link
Owner

Fixed in #38

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.

None yet

2 participants