> On Nov. 8, 2016, 1:44 a.m., Darrel Schneider wrote: > > geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java, > > line 3315 > > <https://reviews.apache.org/r/53557/diff/1/?file=1556244#file1556244line3315> > > > > If pdx.getClassName() is not equals to JSON_CLASSNAME then would it > > make sense to set valueClassName to pdx.getClassName()? That way you could > > have "read-serialized" set to true and still have a meaningful value > > constraint with pdx
No, definitely not. That would cause it to start rejecting PdxInstances of subclasses of the constraint class. For example, if the region is constrained to hold Person objects and a client entered an Employee object (subclass of Person) into the region a server would reject the value if we checked that the PdxInstance's class name had to match Person. It's enough that the original cache that entered the object into the system performed a constraint check on the original object before it was serialized. That's not the case for Json objects coming from the Rest API - they need to be checked and that is the reason for this change. > On Nov. 8, 2016, 1:44 a.m., Darrel Schneider wrote: > > geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java, > > line 3323 > > <https://reviews.apache.org/r/53557/diff/1/?file=1556244#file1556244line3323> > > > > Is ".equals" too specific? You want something like instanceof. For > > example if the value-constraint was "Number" you would want it to be > > satisfied if "valueClassName" was "Integer", "Long", etc. > > > > You could do this with Class.isAssignableFrom(this.valueConstraint) but > > in that case you need to load a class for "valueClassName". One of the > > goals of pdx is that you do not need to be able to load the domain classes > > on the server, so you might not want to do this check. If you leave it with > > just a simple equals check you should document that in the case of pdx the > > value-constraint needs to exactly match (i.e. no instanceof support). In the case of Rest objects there are no classes to perform this kind of check so equals() is the correct operation to use. For non-Pdx objects we already use instanceof checks. For non-Pdx objects we perform no constraint check at all because it's already been performed and we can't be assured that we have classes for the objects held by the PdxInstance anyway. - Bruce ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53557/#review155241 ----------------------------------------------------------- On Nov. 7, 2016, 10:13 p.m., Bruce Schuchardt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53557/ > ----------------------------------------------------------- > > (Updated Nov. 7, 2016, 10:13 p.m.) > > > Review request for geode, Barry Oglesby, Hitesh Khamesra, and Udo Kohlmeyer. > > > Bugs: GEODE-2080 > https://issues.apache.org/jira/browse/GEODE-2080 > > > Repository: geode > > > Description > ------- > > If you set a value constraint on a cache Region you will be unable to store > objects in the region via the Rest API. This change-set modifies > LocalRegion's constraint check to look for a Rest document and use its type > name in the constraint check > > > Diffs > ----- > > geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java > 3873e6e159ebba4c1a288e9fccde5dbabd2a1140 > geode-core/src/test/java/org/apache/geode/pdx/PdxClientServerDUnitTest.java > 9a9680a7e14ddec91f005fa0f0c6c3da8d033df2 > > Diff: https://reviews.apache.org/r/53557/diff/ > > > Testing > ------- > > new test, precheckin > > > Thanks, > > Bruce Schuchardt > >
