> On Feb. 28, 2017, 2:21 p.m., Darrel Schneider wrote: > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/PageEntry.java, > > line 26 > > <https://reviews.apache.org/r/57170/diff/1/?file=1652074#file1652074line26> > > > > I found it confusing PageEntry was DataSerializable. Usually we do not > > want our internal classes to use DataSerializable. > > > > But it looks to me like this class is never serialized. Instead we > > directly call toData and fromData in the PageResults class. > > > > So I think it would be better to make this a class that does not > > support serialization directly. Otherwise someone in the future may be > > tempted to pass an instance of this class to writeObject. > > > > I could be wrong about this and it would be good to check with the > > serialization team to see what they think. > > Dan Smith wrote: > Sure - I just changed the class so it doesn't implement DataSerializable. > Let me know what you think.
Looks good! - Darrel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57170/#review167159 ----------------------------------------------------------- On Feb. 28, 2017, 2:41 p.m., Dan Smith wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57170/ > ----------------------------------------------------------- > > (Updated Feb. 28, 2017, 2:41 p.m.) > > > Review request for geode, Barry Oglesby and Jason Huynh. > > > Repository: geode > > > Description > ------- > > If values on the server are in serialized form, leave them that way and > only deserialize the values on the clients. > > This is implemented by having the LuceneGetPageFunction extract the > serialized value and return the results in a new PageResults class, > which handles the deserialization and upwrapping of the value if > necessary. > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/internal/DataSerializableFixedID.java > 4e45646cf8aba6a66ed77514d270ffadd74d9859 > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java > dd32000c9cbb2525f2bb6edd1744c95cbfbaded3 > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunction.java > f5c2b99e5bcb58997e973bc660c11c3925711bd0 > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/MapResultCollector.java > 9264126b0bc6cdd63f702c5628346516e3979792 > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/PageEntry.java > PRE-CREATION > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/PageResults.java > PRE-CREATION > > geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunctionJUnitTest.java > c62082cbd54be3719530d1f5ccb5dbb653c280e1 > > geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/results/PageEntryJUnitTest.java > PRE-CREATION > > geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/results/PageResultsJUnitTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/57170/diff/ > > > Testing > ------- > > > Thanks, > > Dan Smith > >
