Announcement Announcement Module
Collapse
No announcement yet.
set contains(obj) returns true but remove(obj) fails. Page Title Module
Move Remove Collapse
X
Conversation Detail Module
Collapse
  • Filter
  • Time
  • Show
Clear All
new posts

  • set contains(obj) returns true but remove(obj) fails.

    OK, this is related to my vertex question. But this looks like the brunt of my problem.

    In my User object I have a method to remove the ItemUserSignedUpToBringToEvent from its Set collection. I have implemented ItemUserSignedUpToBringToEvent's equals method to compare nodeIds.

    Code:
    public boolean removeItemSignedUpFor(ItemUserSignedUpToBringToEvent itemSignedUp) {
            return (itemsSignedUpFor.contains(itemSignedUp) &&
                    itemsSignedUpFor.remove(itemSignedUp));
        }
    contains call returns true, but the call to remove fails. Why???

    Here is my equals and hashcode for the ItemUserSignedUpToBringToEvent

    Code:
        @Override
        public boolean equals(Object o) {
            if (this == o) return true;
            if (o == null || getClass() != o.getClass()) return false;
    
            ItemUserSignedUpToBringToEvent signedUpItem = (ItemUserSignedUpToBringToEvent) o;
            if (nodeId == null) return super.equals(o);
            return nodeId.equals(signedUpItem.nodeId);
    
        }
    
        @Override
        public int hashCode() {
            return nodeId != null ? nodeId.hashCode() : super.hashCode();
        }
    In stepping through code, the Spring Data Neo4J class ManagedFieldAccessorSet remove method is returning false.

    Code:
    @Override
        public boolean remove(Object o) {
            if (delegate.remove(o)) {
                update();
                return true;
            }
            return false;
        }
    Thanks

    Mark
    Last edited by bytor99999; May 26th, 2012, 05:40 PM. Reason: added more

  • #2
    I hate to say this but I had to do a workaround. Basically had to rewrite the remove() method of my Set instead. So basically not use the Java algorithm Big O notation for removal in a HashSet, which is backed by a HashMap.

    Instead my workaround is brute force or
    I should say brute recreate Set by copying the elements of the first set that I want to keep into the new Set. But it does the trick for now, I am done spending 2 weeks on this one problem. If you remember a couple months ago I posted about this problem but never got resolved.

    Ugly code. (I guess you could call this a Joel Spolsky Duct Tape programmer solution) I apologize in advanced.

    Code:
    public boolean removeItemSignedUpFor(ItemUserSignedUpToBringToEvent itemSignedUp) {
            boolean results = false;
            HashSet<ItemUserSignedUpToBringToEvent> newSet = new HashSet<ItemUserSignedUpToBringToEvent>();
            if (this.itemsSignedUpFor.contains(itemSignedUp)) {
                for (ItemUserSignedUpToBringToEvent inUser : this.itemsSignedUpFor) {
                    if (!inUser.equals(itemSignedUp)) {
                        newSet.add(inUser);
                    } else {
                        results = true;
                    }
                }
            }
            this.itemsSignedUpFor = newSet;
            return results;
        }
    Mark
    Last edited by bytor99999; May 26th, 2012, 06:00 PM. Reason: forgot a line

    Comment


    • #3
      Michael, I am going to raise a Jira issue for this, although I have no idea what is causing this problem. I have done the same for a few other collections and that same code worked perfectly. And I don't think I could make a test that duplicates it either.

      Thanks

      Mark

      Comment


      • #4
        is there JIRA issue raised for this?
        I also ran into this weird situation - I have parent containing children entitities, and childrenSet.contains(child) == true
        but
        childrenSet.remove(child) == false
        ?1

        Comment


        • #5
          OK, just created the Jira

          https://jira.springsource.org/browse/DATAGRAPH-267

          Are you using simple mapping or the advanced. I am using Simple at the moment, and wondering if that might have an affect.

          But remember, I am using my hack/cludge as a workaround for now until 2.1 is at least RC1 or 2.

          Mark
          Mark

          Comment


          • #6
            Any chance for a test-case-project? I have a test for remove in SDN which works fine. Seems to be dependent on something else like equals/hashCode?

            I don't think super.hashCode and super.equals work very well in this case. As objects can get an id after they were put into the set and then their equals/hashCode might change.

            Comment


            • #7
              Originally posted by bytor99999 View Post
              Here is my equals and hashcode for the ItemUserSignedUpToBringToEvent

              Code:
                  @Override
                  public boolean equals(Object o) {
                      if (this == o) return true;
                      if (o == null || getClass() != o.getClass()) return false;
              
                      ItemUserSignedUpToBringToEvent signedUpItem = (ItemUserSignedUpToBringToEvent) o;
                      if (nodeId == null) return super.equals(o);
                      return nodeId.equals(signedUpItem.nodeId);
              
                  }
              
                  @Override
                  public int hashCode() {
                      return nodeId != null ? nodeId.hashCode() : super.hashCode();
                  }
              In stepping through code, the Spring Data Neo4J class ManagedFieldAccessorSet remove method is returning false.

              Code:
              @Override
                  public boolean remove(Object o) {
                      if (delegate.remove(o)) {
                          update();
                          return true;
                      }
                      return false;
                  }
              Mark,

              The delegate for ManagedFieldAccessorSet should be of type HashSet, and as such, it will look at equals + hashcode for identity. That snippet of code in ManagedFieldAccessorSet is so simple we can ignore it I think.

              So my guess is, there is something subtle going on with your equals and hashcode implementations.
              • try a hashcode implementation of 'return 42;' - not for production use, just to eliminate that as a source of problems
              • for equals, try 'return o instanceof ItemUserSignedUpToBringToEvent && id.equals( (((ItemUserSignedUpToBringToEvent) o).getId()) );' - does that make your test pass?
              • I guess it can be reduced to writing tests for inserting and removing from a HashSet, try writing a unit test like that, to gain confidence your equals and hashcode work independently of SDN

              I just implemented a test to replicate, and actually managed to get equals+hashcode wrong in the first instance, which reproduced your problem Anyway, I got my test passing, so please try these simple remedies, and if it doesn't work I'll have another look.

              Regards,

              Lasse

              Comment


              • #8
                Thanks guys. Lasee, I will test your suggestions. And see if I can isolate my stuff for a good sample.

                Mark

                Comment


                • #9
                  Although, what is odd, is that in many other collections it works. And that all my entity/domain objects have basically the exact same equals/hashcode. I just copied and pasted the code in all my objects, then just had to change the types in the code to match the domain object it was in.

                  Mark

                  Comment


                  • #10
                    Just want to point out that I also created yesterday the issue tied to this (maybe not the best issue title):
                    https://jira.springsource.org/browse/DATAGRAPH-265

                    But I finally think that crux of problem is described in:
                    http://forum.springsource.org/showth...o-lazy-loading

                    Comment


                    • #11
                      OK. Took me sometime, but I have managed to create a "sample" project, many parts from my real app. but the key parts to replicate. So I wrote a test, and while testing to get it to match, I saw something interesting. Now I say interesting because I don't want to just go out and call myself stupid. Especially, since there is still a possibility I am not. as in not stupid.

                      So, what i see at this moment, is that the Collection in discussion here is not "@Fetch"d and In looking at the var values, it appears the collection is empty. So of course an empty collection will always return false for remove().

                      And since I am almost 2 months since I first posted this, remembering back to that day, I am pretty sure that contains() still returned true for me. That was one of the tests I did back then. Split up my line into separate lines for contains and remove and that contains did return true. But contains still should have returned false, if the collection was empty back then.

                      But also in my test project, even though I am sure I save an ItemSignedUp for, it isn't showing up with my User, even if I use template.fetch(user.getItemsSignedUpForEvents());

                      I think I am going to zip up my files and post it to the Jira ticket I opened up which is DATAGRAPH-267, so I guess you beat me to it. Maybe there is still something to this, maybe it is just me.

                      Thanks for all your help.

                      Mark

                      Comment


                      • #12
                        OK, I am not crazy, in my real application, the collection does get populated. So when I was testing back in May, it was populated and called remove.

                        So, I will have to figure out why in my test sample project it isn't getting populated even when I added @Fetch.

                        Mark

                        Comment


                        • #13
                          Basically, sometimes my collection will be empty, sometimes filled based on what happened on previous requests. When I load my User, it is through Spring Security using the User solution in the SDN Guide. So when I have later requests and I need my current User I get the User from the SecurityContext in the Session. So first load of the User when "logging in" the collection will be empty. But later the collection could have entries in it from other actions. Then go to the deleting and get contains to return true and remove to return false. So it would be very difficult to replicate that in a sample app/test.

                          Mark

                          Comment


                          • #14
                            Mark,

                            This is not really an answer, just some thoughts. I think you need to break down what you are trying to do into separate chunks:
                            - user authentication vs domain logic - do you need the same User object for both? If so, I guess you need a full, eagerly-fetched one, because that is what you will likely need for domain stuff - even though you might not need that for authentication
                            - do you need deep, nested equals on your entities, or is it good enough to just compare identities?
                            - when adding new users, attach them to the graph (call save) first, so they get an ID - before you stick them into collections and use them in domain logic

                            I always find simpler/ dumber/ less sophisticated/ less re-used code is easier to reason about and change

                            Comment


                            • #15
                              Originally posted by lassewesth View Post
                              Mark,

                              This is not really an answer, just some thoughts. I think you need to break down what you are trying to do into separate chunks:
                              - user authentication vs domain logic - do you need the same User object for both? If so, I guess you need a full, eagerly-fetched one, because that is what you will likely need for domain stuff - even though you might not need that for authentication
                              - do you need deep, nested equals on your entities, or is it good enough to just compare identities?
                              - when adding new users, attach them to the graph (call save) first, so they get an ID - before you stick them into collections and use them in domain logic

                              I always find simpler/ dumber/ less sophisticated/ less re-used code is easier to reason about and change
                              "I always find simpler/ dumber/ less sophisticated/ less re-used code is easier to reason about and change"

                              That is always my #1 rule, philosophy, motivator. 80% of all software costs is in maintenance. Every decision I make is always based/rooted in reducing maintenance.

                              My User and UserDetails objects are two different objects. However, my UserDetails has a reference to a User object. Just like in the guide book. I have used this approach before and it works great for some session data without dealing directly with the HttpSession.

                              I agree I should be fully populating the User object that is stored inside my UserDetails.

                              equals methods are all defined shallow, no referenced objects should be used in equals. I only use ID, again just like in the SDN guide.

                              "when adding new users, attach them to the graph (call save) first, so they get an ID - before you stick them into collections and use them in domain logic"

                              It isn't actually User objects I am putting into the Collection. But that isn't really the point anyway.

                              Thanks for your insights Lasse.

                              Mark

                              Comment

                              Working...
                              X