> On Dec. 23, 2013, 2:28 a.m., Ali Lown wrote: > > src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java, line > > 220 > > <https://reviews.apache.org/r/16322/diff/8/?file=402000#file402000line220> > > > > Please ensure the wavelet is a root conversation wavelet here, using > > the appropriately named method in IdUtils. > > Frank R. wrote: > It won't be necessary here. This is only a comment. I'll do it for > SolrRobot of the same problem.
All of the code in this function beyond this point assumes that this wavelet is the root conversation wavelet. You should definitely check that this is the case here, using IdUtil.isConversationRootWaveletId. > On Dec. 23, 2013, 2:28 a.m., Ali Lown wrote: > > src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java, lines > > 232-237 > > <https://reviews.apache.org/r/16322/diff/8/?file=402000#file402000line232> > > > > Use the appropriately named method in IdUtils to determine if this is a > > blip document, rather than this (incorrect) hack. > > Frank R. wrote: > I found "conversation".equals(docName) an equivalent to > org.waveprotocol.wave.model.id.IdUtil.isManifestDocument(String) > > For "m/read".equals(docName), I got only this > > org.waveprotocol.wave.model.supplement.WaveletBasedSupplement.READSTATE_DOCUMENT > > Perhaps, I should have covered all the document names listed in > org.waveprotocol.wave.model.supplement.WaveletBasedSupplement > > Anyway, am I supposed to replace the whole condition with this? > > org.waveprotocol.wave.model.id.IdUtil.isConversationRootWaveletId(WaveletId) This section is checking if the document is a blip document or not. The problem is that it fails to checkfor ghost blip ids and legacy blip ids. The correct solution here, is to replace this whole block of code with: if (!IdUtil.isBlipId(docName)) { continue; } > On Dec. 23, 2013, 2:28 a.m., Ali Lown wrote: > > src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java, line > > 275 > > <https://reviews.apache.org/r/16322/diff/8/?file=402000#file402000line275> > > > > We have the actual wavelet name, don't put this string-thing here. > > Frank R. wrote: > Is this equivalent to waveName? > > waveId + "/~/conv+root/" > > I read waveName as the following with breakpoint-and-watch. > local.net/w+sfuoRAt-oCA/local.net/conv+root > > Anyway, this is for recreating the links as you can find by clicking the > Link button on the upper right corner of a wave editor. I always see things > like > > > wave://local.net/w+sfuoRAt-oCA/~/conv+root/b+sfuoRAt-oCB > > http://localhost:9898/waveref/local.net/w+sfuoRAt-oCA/~/conv+root/b+sfuoRAt-oCB > > Perhaps, I should look into the client side (gwt) code, and see how the > link is stringed together. The link normalization code is in wave.client.doodad.link.Link Since you are referring to a blip, you really want to be looking at wave.model.waveref.*. I think WaverefEncoder.encodeToUriPathSegment(new WaveRef(wavelet.getWaveId(), wavelet.getWaveletId(), docName)) gives what you want here. - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16322/#review30803 ----------------------------------------------------------- On Dec. 22, 2013, 5:10 p.m., Frank R. wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16322/ > ----------------------------------------------------------- > > (Updated Dec. 22, 2013, 5:10 p.m.) > > > Review request for wave, Ali Lown, Vicente J. Ruiz Jurado, and Yuri Zelikov. > > > Bugs: WAVE-311 > https://issues.apache.org/jira/browse/WAVE-311 > > > Repository: wave > > > Description > ------- > > For details (issues and commits): > https://github.com/renfeng/wave > > > Diffs > ----- > > run-export.sh d2cddb7 > run-import.sh 45fff8a > server.config.example 19ba8b2 > src/com/google/wave/api/Annotation.java b55f778 > src/org/waveprotocol/box/server/SearchModule.java 2de0ef9 > src/org/waveprotocol/box/server/ServerMain.java b50454d > src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java > PRE-CREATION > src/org/waveprotocol/box/server/robots/agent/welcome/WelcomeRobot.java > 2735940 > src/org/waveprotocol/box/server/waveserver/SimpleSearchProviderImpl.java > ee7093f > src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java > PRE-CREATION > src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java > PRE-CREATION > src/org/waveprotocol/box/server/waveserver/WaveDigester.java b103bbb > > Diff: https://reviews.apache.org/r/16322/diff/ > > > Testing > ------- > > tests on search box > > * in:inbox > * (empty) for all, including waves shared in the domain > * with:@ > * (free texts) > > tests on solr-bot > > * single word > * phrase (quoted with double quotation marks) > * syntax applicable to search box > > > Thanks, > > Frank R. > >
