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

Feature parity/comparison for ActiveRecord enum types #429

Open
crimson-knight opened this issue Jan 11, 2023 · 10 comments
Open

Feature parity/comparison for ActiveRecord enum types #429

crimson-knight opened this issue Jan 11, 2023 · 10 comments

Comments

@crimson-knight
Copy link
Contributor

I'm currently working on syncing a Rails app with a Crystal app using Jennifer and Mosquito to handle my background processing.

I have an ActiveRecord model that uses a hash enum for a particular column, like this:

class MyClass < ApplicationRecord
  # bunch of code here
  enum type: {
    my_type_1: "my type 1",
    my_type_2: "my type 2"
  }
end

I can then use it in this way:

my_class = MyClass.new(type: :my_type_1)

my_class.type # returns: "my_type_1"
my_class.type = "my type 1" # sets the `type` attribute to "my_type_1"
my_class.type = :my_type_1  # sets the `type` attribute to "my_type_1"

This doesn't work in Crystal because Enums can only be integer values and that's fine, but it would be nice to make the experience feels like it's a feature of Jennifer.

I can work my solution into the code base and create a PR (it'll probably be at least a few weeks), but I wanted to see if it's a feature you'd like to see added.

Please let me know if you'd like this type of functionality added to Jennifer :)

@cyangle
Copy link
Contributor

cyangle commented Jan 12, 2023

@crimson-knight Here's my workaround:

enum Gender
  UNSPECIFIED
  FEMALE
  MALE

  def to_s
    super.downcase
  end
end

class User < Jennifer::Model::Base
  with_timestamps

  mapping(
    {
      id:           Primary64,
      name:         String,
      email:        String,
      lock_version: {type: Int64, default: 0_i64},
      gender:       {type: Gender, converter: Jennifer::Model::EnumConverter(Gender), default: Gender::UNSPECIFIED},
      created_at:   {type: Time, default: Time.utc},
      updated_at:   {type: Time, default: Time.utc},
    },
    false
  )
end
pry(bin/console.cr)> u = User.new({name: "", email: ""}, false)
 => #<User:0x7fa6f7d932d0 id: nil, name: "", email: "", lock_version: 0, gender: unspecified, created_at: 2023-01-12 01:00:26.753184361 UTC, updated_at: 2023-01-12 01:00:26.753200496 UTC>
pry(bin/console.cr)> u.gender
 => unspecified
pry(bin/console.cr)> u.gender = :male
 => male
pry(bin/console.cr)> u.gender
 => male
pry(bin/console.cr)> u.gender.class
 => Gender

@crimson-knight
Copy link
Contributor Author

@cyangle that works if the db column is an integer, but I need to store the value as a string (to maintain existing functionality for now)

I ended up with this:

class ActiveRecordEnumConverter
  def self.from_db(result_set : DB::ResultSet, options_tuple : NamedTuple)
    result_value = result_set.read(String)
    PollingPlace.available_types_hash.each do |key, value|
      return key.to_s if value == result_value
    end
  end

  def self.to_db(save_value : String | Nil, options) : String
    save_value = "PS" if save_value.nil?
    new_save_value = "PS"
    PollingPlace.available_types_hash.each do |key, value|
      new_save_value = value if key.to_s == save_value || value == save_value
    end

    new_save_value
  end

  def self.from_hash(hash : Hash(String, Jennifer::DBAny | T), column_name : String, named_tuple : NamedTuple)
    hash[column_name]
  end
end

class PollingPlace < ApplicationRecord
  with_timestamps

  mapping(
    id: Primary64,
    type: { type: String, converter: ActiveRecordEnumConverter, default: "PS" },
    updated_at: Time,
    created_at: Time
  )

  AVAILABLE_TYPES = {
    "precinct_specific" => "PS",
    "early_absentee" => "ES",
    "vote_center" => "VC"
  }

  def self.available_types_hash
    AVAILABLE_TYPES
  end
end

I skipped the support for passing in symbols for now, but I could pretty easily mix that in.

I still think it would be nice to have this mixed into the core of the ORM to make adoption from existing Rails/ActiveRecord projects easier.

@cyangle
Copy link
Contributor

cyangle commented Jan 14, 2023

@crimson-knight It works with string column and postgres enum type column:
You can find more about creating string backed enum column in here

Here's the users schema:

debater_dev=# select * from users;
 id | name |      email       |   gender    | lock_version |         created_at         |         updated_at         
----+------+------------------+-------------+--------------+----------------------------+----------------------------
  1 | test | test@example.com | unspecified |            0 | 2023-01-14 03:05:38.738286 | 2023-01-14 03:05:38.738311
(1 row)

debater_dev=# \d users
                                         Table "public.users"
    Column    |            Type             | Collation | Nullable |              Default              
--------------+-----------------------------+-----------+----------+-----------------------------------
 id           | bigint                      |           | not null | nextval('users_id_seq'::regclass)
 name         | character varying(254)      |           | not null | 
 email        | character varying(254)      |           | not null | 
 gender       | character varying(254)      |           | not null | 'unspecified'::character varying
 lock_version | bigint                      |           | not null | 0
 created_at   | timestamp without time zone |           | not null | 
 updated_at   | timestamp without time zone |           | not null | 
Indexes:
    "users_pkey" PRIMARY KEY, btree (id)
    "lower_case_email" UNIQUE, btree (lower(email::text))
    "users_created_at_idx" btree (created_at)
    "users_updated_at_idx" btree (updated_at)

@cyangle
Copy link
Contributor

cyangle commented Jan 14, 2023

So, basically both postgresql and jennifer treat the column as string column.
You can use integer backed enum column by removing the enum converter from the model.

@crimson-knight
Copy link
Contributor Author

Thanks @cyangle but that doesn't help me here, I have an existing MySQL db using string values for this enum column. That's an existing pattern in Rails/ActiveRecord so I figured it would be a good idea to support it here as well

@cyangle
Copy link
Contributor

cyangle commented Jan 18, 2023

@crimson-knight You might find this useful: crystal-lang/crystal#10281 (comment)

enum PollingType
  PRECINCT_SPECIFIC
  EARLY_ABSENTEE
  VOTE_CENTER

  def self.parse(value) : self
    case value.to_s.upcase
    when "PS", "PRECINCT_SPECIFIC"
      PRECINCT_SPECIFIC
    when "ES", "EARLY_ABSENTEE"
      EARLY_ABSENTEE
    when "VC", "VOTE_CENTER"
      VOTE_CENTER
    else
      raise "Unknown value for PollingType"
    end
  end

  def to_s : String
    case self
    when PRECINCT_SPECIFIC
      "PS"
    when EARLY_ABSENTEE
      "ES"
    when VOTE_CENTER
      "VC"
    else
      raise "Unknown value for PollingType"
    end
  end
end

Jennifer::Model::EnumConverter reads from db with parse and writes to db with to_s.
https://github.com/imdrasil/jennifer.cr/blob/master/src/jennifer/model/enum_converter.cr

@crimson-knight
Copy link
Contributor Author

Thanks, seeing multiple ways to accomplish something is great. But the question still remains, can we add this to Jennifer to make it native behavior? @imdrasil what are your thoughts?

@imdrasil
Copy link
Owner

Hm, it sounds that in your case you actually need to have a slightly complicated logic for a database value transformation rather than actual enum functionality. I see value converting string/integer value to crystal enum during runtime to restrict values that can be used. But I'm not sure is it a good idea to have a built-in utility to transform string value from one format to another. I'd rather use a combination of serialized/decorator and input data transformation to be able to use different value for front-end and back-end. What do you think?

@crimson-knight
Copy link
Contributor Author

@imdrasil yes that's right along the lines of what I was thinking. I'm mostly thinking of a nice easy DSL to use that would make utilizing the feature easier for someone coming from ActiveRecord.

@crimson-knight
Copy link
Contributor Author

I'm still working on this FYI, I'll post a PR with the idea as soon as I can

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

No branches or pull requests

3 participants