feature: has_many & has_one model_labels: [] option (instead of model_class)

[jorroll]

Problem

Currently, when defining a has_many or has_one association, you indicate what the association's targets are using model_class. This can be limiting. For example, given the models below, I want the person's has_many association to target current signup questions AKA nodes with (:`Question::Signup`:`Current`) labels. This could be accomplished by creating a class specifically for this purpose (i.e. Current = Class.new(Question::Signup){mapped_label_name = "Current"}) but the class would only exist to work around neo4jrb's limitations.

class Person
  has_many :out, :signups, type: :SIGNUP, model_class: ???
end

class Question
  self.mapped_label_name = "Question"

  class Signup < Question
    self.mapped_label_name = "Question::Signup"

    class List < Signup
      self.mapped_label_name = "Question::Signup::List"

      class Current < List
        self.mapped_label_name = "Current"
      end

      class Destroyed < List
        self.mapped_label_name = "Destroyed"
      end
    end

    class Shift < Signup
      self.mapped_label_name = "Question::Signup::Shift"

      class Current < Shift
        self.mapped_label_name = "Current"
      end

      class Destroyed < Shift
        self.mapped_label_name = "Destroyed"
      end
    end
  end
end

Proposal

Would you accept a pull request to add a model_labels option to has_many() and has_one()? This option could be used instead of model_class (an ArgumentError would be raised if they both were included) and would indicate the labels to match against. To use the option, you would include an array of strings where each string was formatted like the label part of a cypher query.

  has_many :out, :children, type: nil, model_labels: [":Current:`Question::Signup`", ":Person:Current"]

[cheerfulstoic]

Hey, thanks for this and #1414. I have been putting off looking at them because this is something that I want to proceed with carefully and so I've wanted to have some time to sit and think about it. I've run into such issues either on my team or via issues that people have submitted, and I think if we find the right abstraction we could have a powerful addition to the gem.

My first question is, why have Current / Destroyed labels rather than current and destroyed boolean properties (or a destroyed boolean, perhaps, if the two states are mutually exclusive). I think that labels are often a poor fit for describing the state of things rather than the type of things.

Secondly, why have classes called Question and Signup? Are those things that you plan on querying for as a group (that is, if you did Signup.all you would get both List and Shift nodes). If not, I would suggest making Question and Signup modules and not having inheritance. For example:

module Question
  module Signup
    class List
      self.mapped_label_name = 'Question::Signup::List' # This should actually be the default label anyway for nested `ActiveNode` models
    end
  end
end

I ask these questions because I want first make sure any potential design questions first before deciding on if a new feature is warranted. If you can do everything that you need with the existing gem, then maybe we don't need something new (yet). If we were to do this, it would certainly make more sense on an association rather than the model (as in #1414) because a model defines something more "permanent" and if I were to use the User model it would feel weird if that model also had the Author and Admin labels. With an association you can create as many associations as you want referring to whatever combination of models / relationship / options. You may have already decided the same because you created this issue, but I wanted to put that out there ;)

All of that said, the way in which I've seen a need to be able to have some flexibiliy in the models is because I think sometimes it make sense for labels to map to "behavior" as well as "type". As an example, take these generic models:

# Assume these are all `ActiveNode` models
class Foo
end

class Bar
end

class Baz
end

You might want to be able to apply Ruby modules like Bizzable and WillKlonk to the classes like:

# Assume these are all `ActiveNode` models
class Foo
  include Bizzable
end

class Bar
  include Bizzable
  include WillKlonk
end

class Baz
  include WillKlonk
end

This sort of cross-cutting, duck-typing behavior is something that Neo4j's labels are really fabulous for. Unfortunately I don't think that you can completely take advantage of them in ActiveNode. But if this could be done, you could have something like:

# Returns a set of nodes which could be `Foo` or `Bar`, but we don't care
# because all we care about is that they behave like a `Bizzable` object and
# respond to the methods defined in `Bizzable`
Bizzable.all

I realize that I may have misunderstood your use-case and you might have no need for what I'm talking about, but I wanted to put that all out there to help facilitate the discussion to something that could be useful for everybody ;)

Thanks again!

[jorroll]

Hey @cheerfulstoic. I got pulled into another project so just now getting back to this. A few things:

• I added the unnecessary self.mapped_label_name to be explicit in my issue.
• This example is contrived, but yes, the goal would be to supporting querying for Question.all or Signup.all.
  • Even if this wasn't my goal however, your suggestion to make Question / Signup modules rather than use inheritance seems like a style choice. Or is there a performance reason to avoid inheritance?

Secondly, I disagree with your implication that ActiveNode models shouldn't have multiple labels associated with them

if I were to use the User model it would feel weird if that model also had the Author and Admin labels.

If I constructed the following model

class User
  class Admin < User; end
  class Author < User; end
end

Admin.new will have both user and admin labels. Which is perfectly natural in Neo4j. I.e. a model is, already, not defined by a single label, it is defined by the set of labels associated with it. Here the set was defined using inheritance, but, I would argue, it is perfectly valid to, instead, define the set using a method like self.mapped_label_names (#1414) and/or inheritance.

Thirdly, while I would agree that a state like "on" or "off" should be kept as a property on the node, for "destroyed" vs "not destroyed," specifically, I think labels are appropriate for several reasons:

1. A Car model can be running: true, so it is natural to keep this state as a property on the associated node. With :Car:Destroyed however, I'm not describing the state of the car as being destroyed, I'm describing the state of the node as being destroyed. Put another way (and also pausing to acknowledge that this is, entirely, an argument over semantics), the :Destroyed label is describing database state, rather than object state. It feels appropriate to use a label in this case. (A :Car:Current node could have a destroyed: true property on it which describes the state of the car as having been destroyed, even though the node itself isn't destroyed).
2. I will also want the ability to query on just :Destroyed nodes, most commonly when running chores such as to permanently find and delete old destroyed nodes (i.e. MATCH (n:Destroyed) WHERE n.updated_at < $six_months_ago DELETE n).
3. In my mind, whether a node is destroyed or not is a much larger / more intrinsic state change then simply on vs off. I like getting back a different object type to describe this (i.e. Car::Current vs Car::Destroyed). In many situations, you would only want methods to operate on a non-destroyed node, and a clean way of handling this is to have different object types for non-destroyed vs destroyed and just construct a method to accept / expect non-destroyed type(s).
4. At minimum, a non-destroyed node will respond to .destroy and a destroyed node will respond to .reincarnate / .undestroy, but there can be others as well. It makes sense to have different classes for destroyed vs non-destroyed objects, to cleanly support these differences.

On a more practical level, I originally made the decision to create different current vs destroyed classes because I wanted my associations to only return non-destroyed nodes and, when I was building out this functionality, bug #1413 prevented my scopes from working property. I didn't realize that scopes weren't working because of a bug, all I knew is that they didn't do what I wanted them to do.

The strongest argument I see in favor of describing current vs destroyed nodes using a property rather than labels, is that the property method makes it easier for Neo4jrb to construct optimized queries. Assuming that it's true, Neo4j query performance is affected by label ordering (I haven't had time to test this myself yet), my solution creates a problem in that the current / destroyed labels will always be placed at the front of a query, while they should come last. Of course, your Bizzable and WillKlonk modules example can also run into this problem. E.g.

class Foo
  include WillKlonk
  
  class Bar
    include Bizzable
  end
end

class Baz
  include Bizzable
end

In the above, I think Foo::Bar.all will query on the labels :`Foo::Bar`:Bizzable:Foo:WillKlonk, but the optimal label ordering is actually :`Foo::Bar`:Foo:WillKlonk:Bizzable.

Even in the current version of Neo4jrb, label ordering could be a problem if someone's schema was like so:

class Cartoon
  self.mapped_label_name = 'Cartoon'

  class Animal < Cartoon
    self.mapped_label_name = 'Animal'

    class Dragon < Animal
      self.mapped_label_name = 'Dragon'
    end

    class Monkey < Animal
      self.mapped_label_name = 'Monkey'
    end
  end
end

class Animal
  self.mapped_label_name = 'Animal'

  class Gecko < Animal
    self.mapped_label_name = 'Gecko'
  end
end

In the above, the optimal query for Cartoon::Animal::Dragon.all might (depending on the database) order the labels :Dragon:Cartoon:Animal. Currently, Neo4j would order the query as :Dragon:Animal:Cartoon. Admittedly, this is a contrived example because it can be prevented by not demodulizing the label names. In fact, I can't immediately think of an example where you shouldn't just modulize the label names, so this may not actually be an issue (an exception would be if you wanted Neo4jrb to play nicely with existing databases which were created outside of Ruby and/or without Neo4jrb--in this case it's likely label names would not be modulized).

Anyway, before I go on, what are your thoughts about all this? Ultimately, my particular issue (using :Current label vs current: true property) is arguably a style / personal preference one, as this problem could certainly be solved either way. Given that Neo4jrb's inspiration, ActiveRecord, draws heavily from convention over configuration, I think it would be perfectly understandable to decide something along the lines ~ "In this case, the convention Neo4jrb supports is using properties to solve this problem rather then labels. As such, this isn't a feature we're looking to include at this time."

[jorroll]

I imagine you're just busy with other things (which is fine), but, in case this slipped off your radar: @cheerfulstoic...ping

[cheerfulstoic]

Sorry, yes, busy with other things, unfortunately. It's on my todo list, but I've just been going through things one or two at a time in between other things. Will definitely respond soon

[jorroll]

No worries! Just wanted to check in.

[cheerfulstoic]

I started to read through, but I feel like this is going to be difficult to talk through in a GitHub issue thread. Can we have a voice and/or video chat at some point? Probably the best way to contact me is to send a message using the form on the neo4jrb website

[cheerfulstoic]

Thanks for the chat. I've gotten a bit of time to sit down and think about this.

I'm honestly not really opposed to having a new option for has_one / has_many (though I would call it labels instead of model_labels because it may be referring to multiple models). But I think there is the possibility for a larger feature which could give a smooth path for these use-cases which anybody could use.

This is perhaps a bit rough, but this is the general solution I'm thinking about:

class List < Signup
  include Current # My use-case

  # or...

  conditional_label :Current # Your use-case
end

module Current
  include Neo4j::Label

  # `ActiveSupport` is a dependency of the `neo4j` gem, so we could use `ActiveSupport::Concern` under the covers
  def some_instance_method
  end

  class_methods do
    def some_class method
    end
  end
end

Having the Neo4j::Label included into the Current module could allow you to do things like Current.all / Current.first / Current.where / etc...

In my use-case, a node wouldn't be wrapped as an object of the class List unless it had the labels List, Signup, and Current (ignoring Question for now to keep things simpler). Since it's using the built-in Ruby include the node would have all of the methods defined in the module.

In your use case nodes would be wrapped the way they are now, but if there was also a Current label on a node then the object would get the methods defined in the module.

It's certainly more work than an association option, though I think it could be a pretty awesome feature. What do you think?

[jorroll]

So actually, I think your proposed module setup is already largely doable in Neo4j (though I may have misinterpreted your thoughts). If you create a module which responds to self.mapped_label_name, and you include that module in an ActiveNode class, that class will inherit the module's label (mapped label names are found through ~~ ancestors.respond_to?(:mapped_label_name) call, which includes modules). I make use of this in my app. While I've never done it before, you should also be able to use this strategy to get scopes included in a model, since scopes are also found through ~~ ancestors.respond_to?(:scopes). The only bit that isn't provided automagically by neo4jrb already is the ability to call Module.all / Module.first, but that should be a pretty easy feature to add.

All this being said, I don't think you're proposal addresses my issue. Given models that look like this:

class Question
  class Signup < Question
    class List < Signup
      class Current < List; end

      class Destroyed < List; end
    end

    class Shift < Signup
      class Current < Shift; end

      class Destroyed < Shift; end
    end
  end
end

I want to create an association that links to all current signup questions (something akin to has_one :current_signup_question, model_class: :"Question::Signup::Current". In cypher, this is easy :`Question::Signup`:`Current` . Currently, in neo4jrb though, that association cannot be made unless a Question::Signup::Current class exists. It doesn't look like your proposal helps to solve this problem?

[jorroll]

Maybe I should also add, that all my Current / Destroyed classes are already defined using a simple module include.

Put another way, this:

In my use-case, a node wouldn't be wrapped as an object of the class List unless it had the labels List, Signup, and Current

Is already doable / is already a feature of this gem :)

(I'm actually pretty sure I learned about this feature from advice you gave to someone else in an old issue)

[jorroll]

Maybe you were thinking that, in your scenario, I could create an association directly to the Current module? But that wouldn't address my problem either as I'm trying to create an association to multiple labels :`Question::Signup`:`Current`

[cheerfulstoic]

Right, I forgot about using the self.mapped_label_name in a module. So yeah, it could be pretty easy to wrap it all into a Labels module which automatically does that based on the module's name as well as adding the extra methods (I think that since they are also used for ActiveNode there would need to be some shared place that the logic lives)

And I definitely missed how my solution doesn't apply to your issue. Thinking about it on the fly I could imagine having something like this:

has_one :current_signup_question, model_class: [:'Question::Signup', :Current]

Where Current is a module which responds to mapped_label_name / mapped_label_names. That way you could change the label of the classes / modules and the associations would still work (assuming that you ran migrations to change the labels).

The big problem that I see with that, though, is that the current semantics for passing an array to model_class is that the labels for the association are defined by just one of the models, not all. So to distinguish you might have to have something like this:

has_one :current_signup_question, model_class: [[:'Question::Signup', :Current], :OtherModel]

Which is a bit ugly... Alternatively the model_class option could be used in conjunction with something like the labels option to show which of the included Ruby modules with Neo4j::Label defined in them is used:

# In my case `Current` is always associated with `Signup`, so it would filter by the `Current` label here
has_one :current_signup_question, model_class: :'Question::Signup'

# In your case, you would need to specify that you want to only look for things with a conditional label
has_one :current_signup_question, model_class: :'Question::Signup', conditional_labels: [:Current]

In my case where a module which is always included, I think that Current isn't a good example of a module, by the way. I think it would be something like Fooable

All of this said, I think that having a labels option as you originally proposed would be a perfectly fine thing to add to the gem which doesn't interfere with any of this stuff that I'm proposing and would give people the ability to configure things exactly how they want if the gem doesn't do quite what they want.

[leehericks]

These are the concrete goals I identify in this proposal/discussion: (Goals, e.g. G1)

1. Keep intact the well-understood inheritance/label model
2. Allow optional labels, and the ability to add or remove them
3. Be able to query any nodes which have one or more optional labels
4. Be able to query an ActiveNode for instances which have one or more optional labels
5. Be able to add shared functionality for nodes with one or more optional labels
6. Integrate these features in a way which logically extends and complements the existing framework (easily understandable and usable)

I see this discussion largely as enhancing the framework to use labels for: (Use Cases, e.g. UC1)

1. Cross-cutting concerns / shared code between nodes
2. Polymorphic associations, I.e. defining a protocol for a batch of nodes possibly from many different model classes.
3. Adding labels for tagging, state, or some other developer-defined custom behavior.

Possible ramifications:

1. Increased graph size by indexes created on many labels
2. Performance hits on searching for multiple labels

Please correct me on any misunderstood parts concerning the goals and use cases. The following is my imagined API to take better advantage of labels:

# The following simple implementation meets G1 and G2, UC 3

class Case # :Case is set by default for an active node, is never removed, needs no extra declaration
  include Neo4j::ActiveNode
  
  # This declares an optional label for the model Case, which is applied by default to new instances
  label :active_case, default: true
  label :needs_review

  property :title, type: String
  has_many :out, :comments, type: :CASE_COMMENT, model_class: :Comment
end

# Create a new case
the_case = Case.new # Case.new(active_case: true)
the_case.title = "A case for more labels"
the_case.save #=> (c:Case:ActiveCase {title: "A case for more labels"})

# See all labels
the_case.labels #=> [:Case, :ActiveCase]

# Remove the active_case label (upon persisting call)
the_case.active_case = false
the_case.save #=> (c:Case {title: "A case for more labels"}) CASE CLOSED :P


# The following contemplates G3 and G4
Case.all.where(has_labels: [:ActiveCase, :NeedsReview])

class Case
  # This could be synthesized automatically from the `label :active_case` declaration
  # Matching only :ActiveCase (purposefully named) is MUCH more performant than :Case:ActiveCase
  scope :labelled_active_case, ->{ where(has_labels: [:ActiveCase]) }
end

Case.labelled_active_case
Case.all.has_labels([:ActiveCase, :NeedsReview])

# Let's look at the possibility of ActiveLabel

class Case
  label :needs_review, label_class: :NeedsReview
  # or just?
  label NeedsReview
end

# Something like this which allows class_eval on the models to insert crosscutting properties and methods
module Neo4j::ActiveLabel
  def labels_models(models, &block)
    models.each |m|
      # get class from symbol
      model_class.class_eval &block
    end
  end
end

class NeedsReview
  include Neo4j::ActiveLabel

  labels_models [:Case, :Report] do
    property :reviewed, type: Boolean, default: false
    has_one :in, :reviewer, model_class: Reviewer, origin: :review_items

    def something

    end
  end

  # Some callbacks like before_save, before_remove_label?
end

NeedsReview.labelled_models

NeedsReview.all
NeedsReview.cases
NeedsReview.reports.where(...)

# Polymorphic associations

class Reviewer
  include Neo4j::ActiveNode

  property :name, type: String

  # Notice the consistency of has_labels across all examples. 
  # Should require that the to_node has the labels. Run into issue of removing label now needs
  # to remove these relationships, and then you are have built a lot of complexity.
  # nodes added to this relationship would get the NeedsReview label set
  has_many :out, :review_items, type: :REVIEWING, model_class: nil, has_labels: [:NeedsReview]
end

[leehericks]

Functioning example code:

module Extendable # ActiveNode
  def self.included(base)
    base.extend ClassMethods
  end

  module ClassMethods
    def property(name)
      puts name.to_s + " defined!"
    end

    # label(ActiveLabelClass)
    def extend_with(extension_class)
      if extension_class.extension_defined?
        extension_class.apply_extension self
      end
    end
  end
end

module Extension # ActiveLabel

  def self.included(base)
    base.extend ClassMethods
  end

  module ClassMethods
    @@extension_block = nil

    def extend_instance(&block)
      @@extension_block = block
    end

    def extension_defined?
      @@extension_block != nil
    end

    def apply_extension(to_class)
      to_class.instance_eval(&@@extension_block) unless @@extension_block == nil
    end
  end
end

# Make a dog extension for barking

class DogExtension # Here is our example "label" which would get applied to nodes
  include Extension # ActiveLabel

  extend_instance do # extend_node, because of instance_eval we can call the methods of ActiveNode
    property :color

    def bark
      puts "Woof!"
    end
  end
end

# Make a barking Cat class

class Cat # Here is our example node which would call "label NeedsReview"
  include Extendable # ActiveNode

  extend_with DogExtension
end

# The test

puts "Ever hear a cat bark?"
Cat.bark

[jorroll]

So there are some great ideas here! A lot of this is tangential to my original issue (concerning polymorphic associations, specifically), so let me first respond to that before responding to the other stuff:

Regarding associations with polymorphic labels

I haven't read anything that addresses my issue better than my original proposal, so I'm going to take @cheerfulstoic's blessing and roll with it :)

I think that having a labels option as you originally proposed would be a perfectly fine thing to add to the gem which doesn't interfere with any of this stuff that I'm proposing and would give people the ability to configure things exactly how they want if the gem doesn't do quite what they want.

Regarding the DSL for this feature, I was originally thinking of passing in an array of cypher formatted label strings and then parsing them, something like:

has_one :current_signup_question, node_labels: [":`Question::Signup`:Question:Current", ":OtherModel"]

But testing this out just now in an editor, I find a matrix of label symbols to actually be more readable.

has_many :test, node_labels: [[:"Person::Current", :Person, :Current], [:"Question:Current"]]

I agree that the matrix is a bit ugly, but A) I find it very readable (the format doesn't abstract too far away from the cypher) B) this isn't a feature that people will use often, and if they want to simplify it they can pass in variables.

has_many :test, node_labels: [[:"Person::Current", :Person, :Current], Question::Current.mapped_label_names]

# or

question_labels = Question::Current.mapped_label_names
person_labels = [:"Person::Current", :Person, :Current]

has_many :test, node_labels: [person_labels, question_labels]

I don't like the idea of creating a DSL that is more ActiveNode specific (e.g. model_class: :'Question::Signup', conditional_labels: [:Current]). I find that much harder to understand. This specific example of a different DSL is also bad because, in my case, :Current isn't conditional it is additional. What if I wanted to match on
(model class Question::Signup containing :Current label) OR (Form model)?
It's complicating things by trying to match on the wrong concept --> what I really want to do is match on labels.

Regarding modularizing ActiveNode functionality

I think the idea of creating an official way to extract ActiveNode functionality into composable models is a great idea. It's something I make extensive use of, unofficially, in my app already. I also don't think it should be that difficult, as I believe most of the groundwork for this has been laid. The possibility of making an association directly to one of these modules is also intriguing, as that's not something I've thought about doing before. (Though this possibility also calls into question the name of the model_class: association option.)

@leehericks, thanks for the feedback! Regarding your goal#2, is this a new idea you're introducing? Or were you responding to your understanding of this conversation? In my mind, and in my understanding of @cheerfulstoic's module idea, an ActiveModule (for lack of a better term at this moment) would have one label associated with it. If the ActiveModule was included in a class, that class would always inherit the ActiveModule's label, in the same way as the class inherits any ancestors label and this inheritance would not be optional.

Regarding your second comment with example code, that's an interesting manner of accomplishing extension. I haven't encountered a method like that before. You've defined the reusable code inside of a class that (presumably) makes use of the code, rather than inside a module. I've never seen this before, but I don't think I like it. For one, now Cat.ancestors will return Extendable not DogExtension (I'd prefer it to return DogExtension). In general, I believe the convension Neo4j has embraced is that of ActiveSupport::Concern for code reuse. (or maybe I'm missing an advantage to your method?):

module Animal
  include Neo4j::ActiveModule

  scope :is_scary, -> { where(scary: true) }

  def bark
    puts "Woof!"
  end

  included do
    property :scary, type: Boolean
    property :species
  end
end

class Dog
  include Neo4j::ActiveNode
  include Animal

  property :fluffy_tail, type: Boolean
end

Embracing polymorphism also brings up the ordering of labels. If I remember correctly, @cheerfulstoic, you mentioned that you confirmed label ordering affects count queries? @leehericks, I found the GraphAware article you shared to be very informative. It seems to imply that adding more than one label to a query always decreases query performance. This is something I want to test right now, as I think the results will affect an ActiveModule type feature....BRB

[leehericks]

@thefliik

1. Goal 2: Allow optional labels, and the ability to add or remove them

I understand this conversation to be that you are adding labels to nodes, sometimes optionally. I don't understand the full domain of your graph/project, but I feel really uncomfortable about the way you are overloading labels. I would border on saying you may be abusing the construct. But then again, you seem to have more experience than I do.

The GraphAware article also supports my assertion that you should find a different, more performant way to model your data. If you consider a relational database, one of the downfalls of tables are that they all have to be joined and filtered. When you apply multiple labels, you are causing the indexes for those labels to be unioned. That's why they say make a :BlogPost:ActivePost and just match on :ActivePost.

2. Regarding the second comment with code, I just imagined and implemented this solution

It is a small, working concrete piece of the overall goal. What you missed is that Extendable is just ActiveNode with the label DSL which is completely descriptive and understandable in the domain of Neo4j. I'm not sure why you want to mess with ancestors or inheritance chains.

Going back to my first comment you would of course have the associated methods synthesized. You could call labels to get all labels and check for the existence of one on the node. You could use label= with a boolean to add/remove the label when save is called.

Which also leads to the polymorphic association discussion.

has_many :test, node_labels: [[:"Person::Current", :Person, :Current], [:"Question:Current"]]

I have no understanding of why you have arrays of arrays of labels, nor why you have Person, Current, and Person::Current, but it feels like you're fighting the framework.

[leehericks]

One example of clean API, I will attempt to model more and towards your needs, in hopes of finding consistency.

class Person
  include Neo4j::ActiveNode

  property :name, type: String
  label :current, default: true

  has_many :out, :review_items, type: :REVIEWING, has_labels: [UnderReview]
end

class Question
  include Neo4j::ActiveNode

  property :text, type: String
  label :current, default: true
  label UnderReview
end

class UnderReview
  include Neo4j::ActiveLabel

  labelled_node do
    property :reviewed, type: Bool, default: false
    has_one :in, :reviewer, model_class: :Person, origin: :review_items

    def set_reviewed!
      reviewed = true
      reviewer = nil
      return save
    end
  end
end



p = Person.first
p.labelled_current? # true
p.labels # [:Person, :Current]

q = Question.new(text: "Have many labels could a labeller label, if a labeller could label labels?")
q.labels # [:Question, :Current]
q.reviewer = p
q.save

q.labels # [:Question, :Current, :NeedsReview]

Question.has_labels([:UnderReview]).reviewer # All Persons currently reviewing questions (which are under review)
Person.first.review_items # All models the person is currently reviewing, marked :UnderReview

q1 = Question.first.reviewed? # false

if q1.set_reviewed!
  puts "One down, many more to go!"
  q1.reviewed # true
  q1.labels # [:Question, :Current]
  q1.reviewer # Empty
end

[leehericks]

I would also like to say that I never knew inheritance fused names together to produce one long label. I think adding labels with functionality through ActiveLabel is nice when inheritance isn't needed.

class Character
  include Neo4j::ActiveNode

  property :unicode, type: String
  property :literal, type: String
end

class Kanji < Character
  include Neo4j::ActiveNode

  has_many :out, :readings, model_class: :KanjiReading
end

class Hiragana < Character
  include Neo4j::ActiveNode

  # Something Hiragana specific
end

But with ActiveLabel

class CharacterLabel
  include Neo4j::ActiveLabel

  mapped_label_name = "Character"

  labelled_node do
    property :unicode, type: String
    property :literal, type: String

    def to_s
      literal
    end
  end
end

class Kanji
  include Neo4j::ActiveNode

  label CharacterLabel, removable: false
  has_many :out, :readings, model_class: :KanjiReading
end

class Hiragana
  include Neo4j::ActiveNode
  
  label CharacterLabel, removable: false
  
end

CharacterLabel.all # All characters
Hiragana.first.literal # "あ"
Kanji.all.each {|k| puts k }
Kanji.first.labels # [:Kanji, :Character]

class CharacterList
  include Neo4j::ActiveNode

  has_many :out, :characters, type: :HAS_CHARACTER, has_labels: [CharacterLabel]
end

list = CharacterList.create
list.characters << Kanji.first
list.characters << Hiragana.first
list.characters.count # 2

[leehericks]

class Destroyed
  include Neo4j::ActiveLabel

  labelled_node do
    scope :current, -> { all.where_not(has_labels: [Destroyed])}
  end
end

class Event
  include Neo4j::ActiveNode

  property :name, type: String
  label :destroyed
end

e = Event.create(name: "Ruby Kaigi")
e.labels # [:Event]
e.labelled_destroyed? # false

e.destroyed = true
e.save
e.labels # [:Event, :Destroyed]

Event.all.where_not(has_labels: [:destroyed]) # And this is where you learn negation in Neo4j is expensive
Event.current

[leehericks]

class Dragon
  include Neo4j::ActiveNode
  
  property :color, type: String

  label :animal, removeable: false # implies default: true
  label :cartoon, removeable: false
end

Dragon.first.labels # [:Animal, :Cartoon, :Dragon]

# Ok, you have the three labels you wanted, order doesn't matter, and you can query has_labels
# And for times when you don't want to create an ActiveLabel subclass:
ActiveNode.has_labels(:Cartoon, :Animal).all

class Zoo
  include Neo4j::ActiveNode

  has_many :out, :animals, type: :ZOO_MEMBER, has_labels: [:Animal]

  def escapees
    animals.has_labels([:Escaped])
  end
end

z = Zoo.first
z.animals.each { |a| a.add_label! :Escaped }
z.escapees # Good god, they've all escaped!

z.animals.first.labels # [:Animal, :Cartoon, :Dragon, :Escaped] <- Oh, our dragon! Catch him!
z.animals.first.remove_label! :Escaped

It really needs to be clearly defined what an association which has labels means.

1. Only those nodes with the given labels can be associated with?
2. Nodes added to the association will be given the label?

[leehericks]

Also there should be callbacks for adding and removing the labels.

[jorroll]

So, I really appreciate your taking the time to write all this out and there are some good ideas in here. I also think this issue thread contains two (maybe three) separate conversations.

@cheerfulstoic kinda misunderstood my issue originally, or maybe just got distracted by my unusual example, and started to go off in a tangential direction. I think I got him back on the same page as me now. I think you've read @cheerfulstoic's ideas, and (understandably) started to build off of them. I can't tell if you are independently excited about these ideas (which, if so, is awesome) or if you're trying to help me solve a problem I don't have. I don't mean to be negative, I'd just hate to have you waste your time on a misunderstanding.

So, having said this, lets discuss creating an ActiveLabel like feature (which again, is a tangential issue-->that is, it's kinda related, but not really)

tl;dr;

While I definitely understood what @cheerfulstoic was going for in his example, I didn't appreciate the need for the idea -- or your ActiveLabel idea, until I tried to model Neo4j's example movie database in ActiveNode. At the moment, you can't really. So then I "got it." Looking through what you and @cheerfulstoic put down, I'm not loving the DSL though. I have some ideas which build off of yours and @cheerfulstoic's, and should solve the movie database problem. For my own sanity, let's move the conversation over some sort of module feature to a new issue: #1453.

a few other bits

regarding

I would also like to say that I never knew inheritance fused names together to produce one long label.

Actually, inheritance does not do this. I'm guessing you're referring to Ruby's namespacing. Take this example:

class Person
  class User
  end
end

The User class is namespaced as Person::User, even though User has not inherited Person and shares no code with person. Similarly:

class Person
end
class User < Person
end

In this example User has inherited Person but is namespaced as simply User(not Person::User).

By default, Neo4j, uses modularized label names (so in the first example, the User model would have labels = Person::User. In the second example, the User model would have labels = :Person:User. In this example:

class Person
  class User < Person
  end
end

User would have the labels = :`Person::User`:Person. The docs kinda explain this.

Regarding

When you apply multiple labels, you are causing the indexes for those labels to be unioned. That's why they say make a :BlogPost:ActivePost and just match on :ActivePost.

This makes sense, but it also goes against other performance tests I've read. If true, it also would imply that ActiveNode's habit of querying on all of a model's labels is oftentimes bad (given that, usually, the first, modularized label is all that is needed for the query). I imagine this is something @cheerfulstoic knows about. Regardless, I want to do some more research into this.

[leehericks]

@thefliik can you give some concrete examples of what you don’t like about the DSL? It very much tries to keep in line with what ActiveNode and ActiveRel already are.

[leehericks]

Thanks for clarifying about the module namespacing. Because I didn’t know about it, I was very confused.

[leehericks]

But can you give me a concrete example of why they are useful?

I guess I could put a Reading inside Kanji to get Kanji::Reading label while keeping a short, reusable model name...but I don’t understand the benefit of this.

Simply KanjiReading suffices.

[jorroll]

@leehericks well the more I read over your code, the more I understand it. I think what I wrote down is really similar to what you're going for.

Comments on the DSL:
I'm not loving the required labelled_node do block

class Destroyed
  include Neo4j::ActiveLabel

  labelled_node do
    scope :current, -> { all.where_not(has_labels: [Destroyed])}
  end
end

If we're going to give ActiveLabel its own DSL, I'd say we should try and eliminate the need for a block as much as possible. i.e.

class Destroyed
  include Neo4j::ActiveLabel

  scope :current, -> { all.where_not(has_labels: [Destroyed])}
end

This is basically what I used in #1453. The more I think about it though, I don't like the idea of creating more ActiveLabel specific DSL. I'd vote for simply using ActiveSupport::Concern DSL, which I think most developers will be familiar with

class Destroyed
  include Neo4j::ActiveLabel

  included do
    scope :current, -> { all.where_not(has_labels: [Destroyed])}
  end
end

This format makes it easier for folks to mix in their own logic in the same manner that they do outside of Neo4jrb.

I'm also not loving the need for label methods in the class.

class Dragon
  include Neo4j::ActiveNode
  
  property :color, type: String

  label :animal, removeable: false # implies default: true
  label :cartoon, removeable: false
end

I'd prefer to see someone include the module and then have the module's label automatically added as either an optional label OR automatically added as a required label. Either way, a developer would only need to add some kind of label :animal, optional: true method if they were bucking convention.

[leehericks]

@thefliik I think you have to think this through more deeply.

1. We declare properties and associations. They are concrete, readable schemas. Those calls also synthesize a lot of helper methods. Likewise, a label is a concrete thing applied to a node and making a call like label :animal is the same as property or has_many. Declarative syntax is good and readable.

2. ActiveRecord::Concern is a way to mixin, yes. But this is more than mixing in a module. So if you say include do, you haven't done anything except change labelled_node do and the latter provides context. And that context is well defined by calling label with options as mentioned in 1. And you may want to add logic to your label class which at the label level, not one of the things which are added to labelled nodes.

3. If ActiveLabel classes can respond to callbacks, such as before_save, after_label_removed, etc. then you can't define that in the ActiveLabel class and include it in the ActiveNode without introducing conflicts. ActiveRel has callbacks too and by specifying the rel_class they get hooked into the process. This is no different.

Too much magic is confusing.