7 Replies Latest reply on Oct 6, 2004 3:19 AM by norbert

    RegionManager.checkConflict() may need to add /

    dres1011

      I am using this with hibernate so this may not apply to other usages but shouldn't the evaluation of name conflicts of regions include a slash to separate the child from the parent and check equals separately. i.e:

      Region[] regions = getRegions();
       for(int i=0; i < regions.length; i++) {
       String fqn = regions.getFqn();
       if(myFqn.startsWith(fqn)) { // fqn is a child of myFqn.
       throw new RegionNameConflictException("RegionManager.checkConflict(): new region fqn "
       +myFqn + " is in conflict with current region fqn- " +fqn);
       }
       }
      


      might be better as:
      Region[] regions = getRegions();
       for(int i=0; i < regions.length; i++) {
       String fqn = regions.getFqn();
       if(myFqn.equals(fqn) || myFqn.startsWith(fqn + "/")) { // fqn is a child of myFqn.
       throw new RegionNameConflictException("RegionManager.checkConflict(): new region fqn "
       +myFqn + " is in conflict with current region fqn- " +fqn);
       }
       }
      


        • 1. Re: RegionManager.checkConflict() may need to add /

          Yes, you are right. This can be a problem. :-) It's been fixed.

          Thanks for the effort,

          -Ben

          • 2. Re: RegionManager.checkConflict() may need to add /
            visionlink

            getRegion does a similar test, should it be fixed as well?

            instead of all the (repeated) string concatenations, would it be better to have Region.getFqn() return a string that's always terminated by "/"?

            -l

            • 3. Re: RegionManager.checkConflict() may need to add /
              norbert

              it would be better design if Region.getFqn() returns an Fqn not a String so we could use Fqn.isChildOf(Fqn) instead of String.startsWith()....

              BtW: Fqn are not just concatenated Strings - In the longrun we have to find a way to configure Regions for eviction based on reall (Object-based) Fqn's.

              • 4. Re: RegionManager.checkConflict() may need to add /
                visionlink

                 

                "norbert" wrote:
                it would be better design if Region.getFqn() returns an Fqn not a String so we could use Fqn.isChildOf(Fqn) instead of String.startsWith()....

                BtW: Fqn are not just concatenated Strings - In the longrun we have to find a way to configure Regions for eviction based on reall (Object-based) Fqn's.


                agreed, i was just suggesting a more efficient solution to the current problem that (hopefully) wouldn't require a lot of refactoring.


                • 5. Re: RegionManager.checkConflict() may need to add /
                  visionlink

                  currently, Fqn.isChildOf is broken. if you attempt to check to see if a *parent* is the child of a *child* you'll get an exception:

                  >>> a = Fqn.fromString("/a")
                  >>> b = Fqn.fromString("/a/b")
                  >>> b.isChildOf(a)
                  1
                  >>> a.isChildOf(b)
                  Traceback (innermost last):
                   File "<console>", line 1, in ?
                  java.lang.IndexOutOfBoundsException: Index: 1, Size: 1
                   at java.util.ArrayList.RangeCheck(ArrayList.java:507)
                   at java.util.ArrayList.get(ArrayList.java:324)
                   at org.jboss.cache.Fqn.isChildOf(Fqn.java:183)
                  


                  looks like the check in isChildOf should be:
                  // If they have the same size or the parent node list is longer,
                   // then parentFqn can't be the parent of this Fqn
                   if( parentList.size() >= l.size() ) return false;
                  
                  



                  • 6. Re: RegionManager.checkConflict() may need to add /

                    1. OK, I have now appended automatically the region fqn with a separator so that should solve this problem. A better solution is fqn.toString() is appended automatically with separator as well. But I need to look at it closer.

                    2. Fqn.isChildOf bug is fixed.

                    3. Use of Fqn construction with generic object is good but maybe rare now. Currently, there are places where it does not fit in naturally (e.g., cache loader and evition region). So we will look at it later.

                    I have added couple unit test cases to verify them.

                    Like Bela said, keep the bug report coming, guys! :-)


                    Thanks,

                    -Ben

                    • 7. Re: RegionManager.checkConflict() may need to add /
                      norbert

                      Ben, I'm using generic Objects in Fqn's in the DistributedState-Implementation I'm currently working on. If I stay on this path this will not just be rarely used case....

                      I know it's not easy implement configuration for eviction-policies that support this or cache-loaders, but I'm shure we can solve this.