thomasmueller commented on code in PR #1180:
URL: https://github.com/apache/jackrabbit-oak/pull/1180#discussion_r1372689153
##########
oak-lucene/src/test/java/org/apache/jackrabbit/oak/composite/blueGreen/CustomizedIndexTest.java:
##########
@@ -211,4 +233,25 @@ private void createFolders() throws IOException {
indexDir = tempDir.newFolder("index");
}
+ private static String getActiveLuceneIndexes(Persistence p) {
+ return getActiveLuceneIndexes(p.getCompositeNodestore(),
p.getMountInfoProvider());
+ }
+
+ private static String getActiveLuceneIndexes(NodeStore ns,
MountInfoProvider m) {
+ ArrayList<String> list = new ArrayList<>();
+ IndexInfoServiceImpl indexService = new IndexInfoServiceImpl(ns,
+ new IndexPathServiceImpl(ns, m));
+ for (IndexInfo info : indexService.getAllIndexInfo()) {
+ if (!LuceneIndexConstants.TYPE_LUCENE.equals(info.getType())) {
+ continue;
+ }
+ if (!info.isActive()) {
+ continue;
+ }
+ list.add(info.getIndexPath());
+ }
+ Collections.sort(list);
+ return list.toString();
+ }
Review Comment:
By using string comparison (assertEquals with Strings) It is very easy to
fix the test if the returned value doesn't match the expected value.
##########
oak-lucene/src/test/java/org/apache/jackrabbit/oak/composite/blueGreen/CustomizedIndexTest.java:
##########
@@ -100,6 +112,8 @@ private void compositeLibs2() throws Exception {
"/jcr:root//*[@foo] order by @jcr:path",
"test-2",
"[/content/test, /libs/test2]");
+ assertEquals("[/oak:index/lucene, /oak:index/test-2]",
Review Comment:
Sure, I can add such a test. I don't see how this would improve code
coverage or so, but it's easy, by just removing the "lucene" index.
##########
oak-lucene/src/test/java/org/apache/jackrabbit/oak/composite/blueGreen/CustomizedIndexTest.java:
##########
@@ -211,4 +233,25 @@ private void createFolders() throws IOException {
indexDir = tempDir.newFolder("index");
}
+ private static String getActiveLuceneIndexes(Persistence p) {
+ return getActiveLuceneIndexes(p.getCompositeNodestore(),
p.getMountInfoProvider());
+ }
+
+ private static String getActiveLuceneIndexes(NodeStore ns,
MountInfoProvider m) {
+ ArrayList<String> list = new ArrayList<>();
+ IndexInfoServiceImpl indexService = new IndexInfoServiceImpl(ns,
+ new IndexPathServiceImpl(ns, m));
+ for (IndexInfo info : indexService.getAllIndexInfo()) {
+ if (!LuceneIndexConstants.TYPE_LUCENE.equals(info.getType())) {
+ continue;
+ }
+ if (!info.isActive()) {
+ continue;
+ }
+ list.add(info.getIndexPath());
+ }
+ Collections.sort(list);
+ return list.toString();
+ }
Review Comment:
Whether lists of strings are compared or string representations of lists of
strings... I don't see what difference it would make... But OK, I'll change it.
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexInfoServiceImpl.java:
##########
@@ -62,11 +63,21 @@ public IndexInfoServiceImpl(NodeStore nodeStore,
IndexPathService indexPathServi
@Override
public Iterable<IndexInfo> getAllIndexInfo() {
+ HashSet<String> allIndexes = new HashSet<>();
+ indexPathService.getIndexPaths().forEach(allIndexes::add);
+ HashSet<String> activeIndexes = new HashSet<>();
+ if (indexPathService.getMountInfoProvider().hasNonDefaultMounts()) {
+ IndexName.filterReplacedIndexes(allIndexes, nodeStore.getRoot(),
true).forEach(activeIndexes::add);
+ } else {
+ activeIndexes.addAll(allIndexes);
+ }
return
Iterables.filter(Iterables.transform(indexPathService.getIndexPaths(), new
Function<String, IndexInfo>() {
@Override
public IndexInfo apply(String indexPath) {
try {
- return getInfo(indexPath);
+ IndexInfo info = getInfo(indexPath);
+ info.setActive(activeIndexes.contains(indexPath));
+ return info;
Review Comment:
I don't think so, but let's be careful...
##########
oak-lucene/src/test/java/org/apache/jackrabbit/oak/composite/blueGreen/CustomizedIndexTest.java:
##########
@@ -211,4 +233,25 @@ private void createFolders() throws IOException {
indexDir = tempDir.newFolder("index");
}
+ private static String getActiveLuceneIndexes(Persistence p) {
+ return getActiveLuceneIndexes(p.getCompositeNodestore(),
p.getMountInfoProvider());
+ }
+
+ private static String getActiveLuceneIndexes(NodeStore ns,
MountInfoProvider m) {
+ ArrayList<String> list = new ArrayList<>();
+ IndexInfoServiceImpl indexService = new IndexInfoServiceImpl(ns,
+ new IndexPathServiceImpl(ns, m));
+ for (IndexInfo info : indexService.getAllIndexInfo()) {
+ if (!LuceneIndexConstants.TYPE_LUCENE.equals(info.getType())) {
+ continue;
+ }
+ if (!info.isActive()) {
+ continue;
+ }
+ list.add(info.getIndexPath());
+ }
+ Collections.sort(list);
+ return list.toString();
+ }
Review Comment:
Why would it be less error prone?
##########
oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/IndexName.java:
##########
@@ -33,16 +33,8 @@
import org.slf4j.LoggerFactory;
/**
- * An index name, which possibly contains two version numbers: the product
- * version number, and the customer version number.
- *
- * The format of an index node name is:
- * - The name of the index,
- * - optionally a dash ('-') and the product version number,
- * - optionally "-custom-" and the customer version number.
- *
- * If the node name doesn't contain version numbers / dashes, then version 0 is
- * assumed (for both the product version number and customer version number).
+ * @deprecated
+ * Use oak-core org.apache.jackrabbit.oak.plugins.index.IndexName instead.
Review Comment:
I think it doesn't matter: the compiler and IDE does see the deprecation.
That's probably due to the history of Java: the annotation was added later. But
I can add the annotation as well, no issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
