This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 257567 - Make the maven.indexer query spi available for implementations from other modules
Summary: Make the maven.indexer query spi available for implementations from other mod...
Status: RESOLVED FIXED
Alias: None
Product: projects
Classification: Unclassified
Component: Maven (show other bugs)
Version: 8.1
Hardware: All All
: P3 normal (vote)
Assignee: Tomas Stupka
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-14 14:44 UTC by Tomas Stupka
Modified: 2016-05-02 02:00 UTC (History)
4 users (show)

See Also:
Issue Type: TASK
Exception Reporter:


Attachments
sugested changes in api/spi (26.84 KB, patch)
2016-01-14 15:40 UTC, Tomas Stupka
Details | Diff
new patch (120.78 KB, patch)
2016-03-14 17:46 UTC, Tomas Stupka
Details | Diff
patch (123.15 KB, patch)
2016-04-26 15:11 UTC, Tomas Stupka
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tomas Stupka 2016-01-14 14:44:09 UTC
Reproducibility: Happens every time

currently the package o.n.m.maven.indexer is private and it is not possible to provide third party implementations of indexer xxxQueries.
Comment 1 Tomas Stupka 2016-01-14 15:40:39 UTC
see the attached patch for changes in the o.n.m.maven.indexer.spi/api packages
- o.n.m.maven.indexer.spi was made friend 
- QueryRepositories.Result constructor got public access 

what stays open is how the indexer api should work when there will be more than one query provider available and in what extend it should be reflected in the UI. 
The actual state is that the service with the lowest position will take precedence.
Comment 2 Tomas Stupka 2016-01-14 15:40:46 UTC
Created attachment 158130 [details]
sugested changes in api/spi
Comment 3 emi 2016-01-20 11:05:38 UTC
The patch is sufficient for a 3rd party implementation.

Of course, my module still needs to be added as a friend of maven.indexer.

What I think would be needed is a simple way of canceling a download. A basic SPI such as this would do:

public interface VetoableRepositoryDownloadListener {

    /**
     * Veto a remote repository download
     * @param repository the repository soon to be downloaded
     * @return true is the download is vetoed
     */
    boolean vetoableRemoteDownload(RepositoryInfo repository);
}

then in NexusRepositoryIndexerImpl indexLoadedRepo(), just before a download is started (after "if (repo.isRemoteDownloadable())") you could just check if the download is vetoed:

if (Lookup.getDefault().lookupAll(VetoableRepositoryDownloadListener.class).stream().anyMatch(veto -> veto.vetoableRemoteDownload(repo))) {
    LOGGER.log(Level.FINE, "Skipping download for Remote Repository: {0}", repo.getId());
    return;
}

Then in my module I can just veto the download is the repository is 'central'.

I still have to look into fixing AddDependencyPanel and lazy-load the artifact version children nodes.
Comment 4 Tomas Stupka 2016-01-27 09:53:36 UTC
NexusRepositoryIndexerImpl handles the index download und unpack so maybe
NRII.indexLoadedRepo should not be invoked at all and this should be handled
otherwise and elsewhere.
currently indexLoadedRepo is triggered when:
- update index is manually invoked by user (services > maven repos)
- a new repository is added 
- automatic repository update

A proper solution kind of depends on how the IDE should behave if there is more
then one set of query impls. 

1.) your module ( or maybe some another alternative module? ) takes completely
over for a particular repository. Bad luck for the user if some queries aren't
provided and the respective features aren't covered. (Will have to live with it
or completely fall back on the the build-in download based indexer.) We have to
ensure that missing query impls won't cause any mis-behavior in the IDE. 

how should the user be able to manage what provider handles a repository?
1a.) if the module is installed it always comes first. to fallback on the build-in 
indexer would mean to deactivate/uninstall it. 
1b.) some settings UI 

2.) hybrid mode where netbeans build-in queries would always work as a fallback
in case no other implementation is available, but this sounds messy and not sure 
how it should be properly handled in the UI.

3.) ...

Any ideas/thoughts on this?

also note, that there already is a cli switch
-J-Dmaven.indexing.doNotAutoIndex={semicolon separated list of repo id-s}. 
This avoids automatic index updates, but the manual update would still work 
You can also set it programaticaly. ... not ideal, but for the time being it
might be of some help ...
Comment 5 emi 2016-02-05 20:54:18 UTC
Ideally my Maven Central Search module would take ownership of the 'central' repository and provide all queries but until that is done a hybrid approach (with a fallback on the NetBeans queries) seems to work.

Because I control right now the queries delegator (there is no such notion in the existing NetBeans code which assumes a single query implementation) it's easy for me to disable the remote search feature at runtime without disabling the whole module.

But indeed, if more modules were to exist this trick wouldn't work and the user would have to disable the module.

I wasn't aware of the maven.indexing.doNotAutoIndex flag! It's no API but since I can change it programmatically it should be sufficient for now.

I should underline that my main use-case is to get rid of the Maven Central index download and to search using search.maven.org for the few times a day/a week I need to do it.

I am almost always online and the index download is more disruptive than a potential search failure would be when network was down. Although for some users the index download might be about the wasted bandwidth, for others it's about the interruption, disk usage and disk trashing.

While some companies might use their intranet Nexus servers or something, Maven Central is unavoidable...

Generally speaking if a Search Plugin were to exist for a given Maven Repository Manager I assume it would take ownership of all repositories served by the same Repository Manager.

So then the UI wouldn't have to be about a [provider] <-> [repository] mapping, but perhaps about [provider] <-> [repository manager kind] mappings which are inherited by default and only then allow the user to customize [provider] <-> [repository] (like we allow font/colors customizations I guess).

Anyhow, even in this UI, Maven Central needs probably some dedicated space.

Could you tell me when your patch will be committed?
Comment 6 Tomas Stupka 2016-02-15 12:41:18 UTC
> Ideally my Maven Central Search module would take ownership of the 'central' repository and 
> provide all queries but until that is done a hybrid approach 
> (with a fallback on the NetBeans queries) seems to work.
...
> I should underline that my main use-case is to get rid of the Maven Central index download and 
> to search using search.maven.org for the few times a day/a week I need to do it.
this conflicts with the idea to fallback on the existing impl (which is based on a downloaded index), 
or am i missing something?
Comment 7 Tomas Stupka 2016-03-14 17:46:27 UTC
Created attachment 158854 [details]
new patch

one more try:

see o.n.m.maven.indexer.spi.RepositoryIndexQueryProvider
implement it and based on RepositoryInfo determine if the queries for a repository are handled or not. 
not implemented queries will be handled in the IDE as if they would return an emty result.

see o.n.m.maven.indexer.api.RepositoryIndexer
the RepositoryIndexer.indexRepo() call won't trigger the remote index download and processing for repositories handled by a RepositoryIndexQueryProvider impl other then the default build-in.

if there is no RepositoryIndexQueryProvider impl handling a repository then the current default implementation will jump in (index download and queries).

please let me know if you are still on this and if it suits your needs.
Comment 8 emi 2016-03-31 18:22:22 UTC
> please let me know if you are still on this and if it suits your needs.

For some reason I'm not receiving email notifications when you post Bugzilla comments.

I was just about to send you an email and ask for updates on this issue but I figured I should double-check the issue number first.

I'll read you post and check out your new patch and send you an update this week.

Sorry for the long delay!

PS: Ooh... it's because I'm not in the CC list.
Comment 9 emi 2016-04-24 21:51:31 UTC
This version of the patch is usable too (I had to tweak it a bit to work on the latest main-silver).

I see RepositoryIndexQueryProvider.getBaseQueries is mandatory which is a bit more work. Looking at BaseQueries I've noticed that:

* filterGroupIds seems to be unused except in NexusRepositoryIndexerImplTest

* filterArtifactIdForGroupId seems to be unused.

* getGAVsForPackaging seems to be only used by the AddDependencyPanel and only for NBM artifacts

* getVersions seems to be a subset of GenericFindQuery.

* getRecords also seems like something GenericFindQuery could provide.

* filterPluginGroupIds is used by MavenProjectGrammar and I see this annotation that GrammarQuery.queryValues "Performs fast up to 300 ms.". So I guess filterPluginGroupIds has some unspoken QoS expectations that might fail if I have to do a network roundtrip.

* filterPluginArtifactIds is also used by MavenProjectGrammar

* even getGroups is called by MavenProjectGrammar

So while for my use-case of having at least the GenericFindQuery I don't mind returning a semi-empty BaseQueries impl, I wonder if BaseQueries doesn't need some cleanup too:

* getVersions / getRecords could become part of some SimpleFindQuery

* the unused methods removed

* the time-restricted filterPluginGroupIds / filterPluginArtifactIds should get a better documentation
Comment 10 Tomas Stupka 2016-04-26 14:36:19 UTC
> * the unused methods removed
will remove the obsolete methods. thanks

> * getVersions / getRecords could become part of some SimpleFindQuery
lets keep them for convenience. Otherwise SimpleFindQuery could replace other calls as well and also would become mandatory.

> * the time-restricted filterPluginGroupIds / filterPluginArtifactIds should get a better documentation
yes, at least the madatory BaseQuery should be better documented. 
regarding those filterXXX methods - called from code completion in the pom editor. A "Downloading..." tag is shown until the result is available, but the UI stays responsible. 

generally speaking, most of the calls are made from the user interface and while a reasonable response time is expected, it should not cause any UI freezing etc. In the worst case the users patience will be killed ...
Comment 11 Tomas Stupka 2016-04-26 15:11:24 UTC
Created attachment 159443 [details]
patch
Comment 12 Tomas Stupka 2016-04-29 11:03:21 UTC
fixed in jet-main #c4b8a86c8069
Comment 13 Quality Engineering 2016-05-02 02:00:39 UTC
Integrated into 'main-silver', will be available in build *201605020002* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)

Changeset: http://hg.netbeans.org/main-silver/rev/c4b8a86c8069
User: Tomas Stupka <tstupka@netbeans.org>
Log: Issue #257567 - Make the maven.indexer query spi available for implementations from other modules