----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57170/#review167159 -----------------------------------------------------------
geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/PageEntry.java (line 26) <https://reviews.apache.org/r/57170/#comment239332> 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. - Darrel Schneider On Feb. 28, 2017, 1:47 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, 1:47 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 > >
