matthiasblaesing commented on code in PR #5922:
URL: https://github.com/apache/netbeans/pull/5922#discussion_r1224666872
##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/MinimalArtifactInfoRemoteIndexCreator.java:
##########
Review Comment:
I would remove the uncommented code. I see where it is coming from, but even
now there is a difference between this implementation and the implementation in
the `maven-indexer` repository.
##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/NexusRepositoryIndexerImpl.java:
##########
@@ -554,14 +561,19 @@ private void indexLoadedRepo(final RepositoryInfo repo,
boolean updateLocal) thr
} else {
iur.setThreads(1);
}
-
- NotifyingIndexCreator nic = null;
- for (IndexCreator ic : indexingContext.getIndexCreators())
{
- if (ic instanceof NotifyingIndexCreator) {
- nic = (NotifyingIndexCreator) ic;
- break;
- }
+ if (RepositoryPreferences.getIndexDateCutoffFilter() > 0) {
+ Instant cutoff = ZonedDateTime.now()
+
.minusYears(RepositoryPreferences.getIndexDateCutoffFilter()).toInstant();
+ iur.setExtractionFilter(doc -> {
+ IndexableField date = doc.getField("m"); //
usually never null
Review Comment:
Together with an additional import `import static
org.apache.maven.index.creator.MinimalArtifactInfoIndexCreator.FLD_LAST_MODIFIED`.
This would be more readable:
```suggestion
IndexableField date =
doc.getField(FLD_LAST_MODIFIED.getKey()); // usually never null
```
##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/MinimalArtifactInfoRemoteIndexCreator.java:
##########
@@ -0,0 +1,204 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.maven.indexer;
+
+import org.apache.lucene.document.Document;
+import org.apache.maven.index.ArtifactAvailability;
+import org.apache.maven.index.ArtifactInfo;
+import org.apache.maven.index.creator.MinimalArtifactInfoIndexCreator;
+
+/**
+ * Creates compact remote repository indices by discarding less important
fields,
+ * or fields which can be easily substituted by online search services.
+ *
+ * @author mbien
+ */
+final class MinimalArtifactInfoRemoteIndexCreator extends
MinimalArtifactInfoIndexCreator {
+
+ private static final char FS = ArtifactInfo.FS.charAt(0);
+
+ static {
+ if (ArtifactInfo.FS.length() != 1) {
+ throw new IllegalStateException("field format changed");
+ }
+ }
+
+ @Override
+ public void updateDocument(ArtifactInfo ai, Document doc) {
+ String info = ArtifactInfo.nvl(ai.getPackaging())
+ + ArtifactInfo.FS
+ + ai.getLastModified()
+ + ArtifactInfo.FS
+ + ai.getSize()
+ + ArtifactInfo.FS
+ + ai.getSourcesExists().toString()
+ + ArtifactInfo.FS
+ + ai.getJavadocExists().toString()
+ + ArtifactInfo.FS
+ + ai.getSignatureExists().toString()
+ + ArtifactInfo.FS
+ + ai.getFileExtension();
+
+ doc.add(FLD_INFO.toField(info));
+
+ doc.add(FLD_GROUP_ID_KW.toField(ai.getGroupId()));
+ doc.add(FLD_ARTIFACT_ID_KW.toField(ai.getArtifactId()));
+ doc.add(FLD_VERSION_KW.toField(ai.getVersion()));
+
+ // V3
+ doc.add(FLD_GROUP_ID.toField(ai.getGroupId()));
+ doc.add(FLD_ARTIFACT_ID.toField(ai.getArtifactId()));
+ doc.add(FLD_VERSION.toField(ai.getVersion()));
+ doc.add(FLD_EXTENSION.toField(ai.getFileExtension()));
+
+ if (ai.getName() != null) {
+ doc.add(FLD_NAME.toField(ai.getName()));
+ }
+
+// if (ai.getDescription() != null) {
+// doc.add(FLD_DESCRIPTION.toField(ai.getDescription()));
+// }
+
+ if (ai.getPackaging() != null) {
+ doc.add(FLD_PACKAGING.toField(ai.getPackaging()));
+ }
+
+ if (ai.getClassifier() != null) {
+ doc.add(FLD_CLASSIFIER.toField(ai.getClassifier()));
+ }
+
+// if (ai.getSha1() != null) {
+// doc.add(FLD_SHA1.toField(ai.getSha1()));
+// }
+ }
+
+ @Override
+ public boolean updateArtifactInfo(Document doc, ArtifactInfo ai) {
+ boolean res = false;
+
+ String uinfo = doc.get(ArtifactInfo.UINFO);
+
+ if (uinfo != null) {
+
+ int start = 0;
+ int end = uinfo.indexOf(FS);
+ ai.setGroupId(uinfo.substring(start, end));
+
+ start = end + 1;
+ end = uinfo.indexOf(FS, start);
+ ai.setArtifactId(uinfo.substring(start, end));
+
+ start = end + 1;
+ end = uinfo.indexOf(FS, start);
+ ai.setVersion(uinfo.substring(start, end));
+
+ start = end + 1;
+ end = uinfo.indexOf(FS, start);
+ if (end == -1) {
+ end = uinfo.length();
+ }
+ ai.setClassifier(ArtifactInfo.renvl(uinfo.substring(start, end)));
+
+ if (end < uinfo.length()) {
+ start = end + 1;
+ end = uinfo.length();
+ ai.setFileExtension(uinfo.substring(start, end));
+ }
+
+ res = true;
+ }
+
+ String info = doc.get(ArtifactInfo.INFO);
+
+ if (info != null) {
+
+ int start = 0;
+ int end = info.indexOf(FS);
+ ai.setPackaging(ArtifactInfo.renvl(info.substring(start, end)));
Review Comment:
Instead of using indexOf+substring to split the `info` string, I would use
String#split and work on the resulting String array. From my pov this would
improve readability.
##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/NexusRepositoryIndexerImpl.java:
##########
@@ -359,24 +365,21 @@ private boolean loadIndexingContext(final RepositoryInfo
info) throws IOExceptio
}
LOGGER.log(Level.FINE, "Loading Context: {0}", info.getId());
- List<IndexCreator> creators = new ArrayList<>();
- try {
- for (IndexCreator creator :
embedder.lookupList(IndexCreator.class)) {
- if (OsgiArtifactIndexCreator.ID.equals(creator.getId())) {
- continue; //we are no interested in osgi related
content in lucene documents or ArtifactInfo objects.
- //they take up a lot of memory and we never query them
AFAIK. (import/export packages can take up to 300k
- //239915, 240150 + according to my knowledge we don't
expose any api that would allow 3rd party plugins to query the osgi stuff
- }
- creators.add(creator);
- }
- } catch (ComponentLookupException x) {
- throw new IOException(x);
- }
- if (info.isLocal()) { // #164593
- creators.add(new ArtifactDependencyIndexCreator());
- creators.add(new ClassDependencyIndexCreator());
+ List<IndexCreator> creators;
+ if (info.isLocal()) {
+ creators = List.of(
+ new JarFileContentsIndexCreator(),
+ new MinimalArtifactInfoIndexCreator(),
+ new MavenArchetypeArtifactInfoIndexCreator(),
+ new MavenPluginArtifactInfoIndexCreator(),
+ new ArtifactDependencyIndexCreator(),
+ new ClassDependencyIndexCreator()
+ );
} else {
- creators.add(new NotifyingIndexCreator());
+ creators = List.of(
Review Comment:
I think we should limit this optimization to `central`. If I remember
correctly the remote query support is only supported for that repository.
##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/NexusRepositoryIndexerImpl.java:
##########
@@ -554,14 +561,19 @@ private void indexLoadedRepo(final RepositoryInfo repo,
boolean updateLocal) thr
} else {
iur.setThreads(1);
}
-
- NotifyingIndexCreator nic = null;
- for (IndexCreator ic : indexingContext.getIndexCreators())
{
- if (ic instanceof NotifyingIndexCreator) {
- nic = (NotifyingIndexCreator) ic;
- break;
- }
+ if (RepositoryPreferences.getIndexDateCutoffFilter() > 0) {
+ Instant cutoff = ZonedDateTime.now()
+
.minusYears(RepositoryPreferences.getIndexDateCutoffFilter()).toInstant();
Review Comment:
```suggestion
.minusYears(RepositoryPreferences.getIndexDateCutoffFilter())
.toInstant();
```
##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/NexusRepositoryIndexerImpl.java:
##########
@@ -359,24 +365,21 @@ private boolean loadIndexingContext(final RepositoryInfo
info) throws IOExceptio
}
LOGGER.log(Level.FINE, "Loading Context: {0}", info.getId());
- List<IndexCreator> creators = new ArrayList<>();
- try {
- for (IndexCreator creator :
embedder.lookupList(IndexCreator.class)) {
- if (OsgiArtifactIndexCreator.ID.equals(creator.getId())) {
- continue; //we are no interested in osgi related
content in lucene documents or ArtifactInfo objects.
- //they take up a lot of memory and we never query them
AFAIK. (import/export packages can take up to 300k
- //239915, 240150 + according to my knowledge we don't
expose any api that would allow 3rd party plugins to query the osgi stuff
- }
- creators.add(creator);
- }
- } catch (ComponentLookupException x) {
- throw new IOException(x);
- }
- if (info.isLocal()) { // #164593
- creators.add(new ArtifactDependencyIndexCreator());
- creators.add(new ClassDependencyIndexCreator());
+ List<IndexCreator> creators;
+ if (info.isLocal()) {
+ creators = List.of(
+ new JarFileContentsIndexCreator(),
+ new MinimalArtifactInfoIndexCreator(),
+ new MavenArchetypeArtifactInfoIndexCreator(),
+ new MavenPluginArtifactInfoIndexCreator(),
+ new ArtifactDependencyIndexCreator(),
+ new ClassDependencyIndexCreator()
+ );
} else {
- creators.add(new NotifyingIndexCreator());
+ creators = List.of(
+ new MinimalArtifactInfoRemoteIndexCreator(),
+ new NotifyingIndexCreator()
+ );
Review Comment:
I understand, that `MavenPluginArtifactInfoIndexCreator` and
`MavenArchetypeArtifactInfoIndexCreator` are dropped in addition to the
`JarFileContentsIndexCreator` and `OsgiArtifactIndexCreator`, as both need the
artifact and thus won't work for remote repositories?
##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/NexusRepositoryIndexerImpl.java:
##########
@@ -417,7 +420,11 @@ private boolean loadIndexingContext(final RepositoryInfo
info) throws IOExceptio
}
}
- private @CheckForNull IteratorSearchResponse repeatedPagedSearch(Query q,
final List<IndexingContext> contexts, int count) throws IOException {
+ private @CheckForNull IteratorSearchResponse repeatedPagedSearch(Query q,
IndexingContext context, int count) throws IOException {
+ return repeatedPagedSearch(q, List.of(context), count);
Review Comment:
Is using `List#of` safe here? If I remember correctly
`Collections#singletonList` can handle `null`, while `List#of` will throw a
`NullPointerException`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
