App Manager does not properly handle installation error when not connected to the internet

ERROR: Resolver: Install error - com.lorainelab.protannot
java.io.IOException: Error accessing http://bioviz.org/igbserver/optional/protannot-8.5.4.jar
at org.apache.felix.bundlerepository.impl.FileUtil.openURL(FileUtil.java:217)
at org.apache.felix.bundlerepository.impl.FileUtil.openURL(FileUtil.java:199)
at org.apache.felix.bundlerepository.impl.ResolverImpl.deploy(ResolverImpl.java:629)
at com.lorainelab.igb.plugins.BundleActionManager.installBundle(BundleActionManager.java:122)
at com.lorainelab.igb.plugins.BundleActionManager.access$000(BundleActionManager.java:37)
at com.lorainelab.igb.plugins.BundleActionManager$1.get(BundleActionManager.java:101)
at com.lorainelab.igb.plugins.BundleActionManager$1.get(BundleActionManager.java:96)
at java.util.concurrent.CompletableFuture$AsyncSupply.run(Unknown Source)
at java.util.concurrent.CompletableFuture$AsyncSupply.exec(Unknown Source)
at java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(Unknown Source)
at java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
at java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
Caused by: java.net.UnknownHostException: bioviz.org
at java.net.AbstractPlainSocketImpl.connect(Unknown Source)
at java.net.PlainSocketImpl.connect(Unknown Source)
at java.net.SocksSocketImpl.connect(Unknown Source)
at java.net.Socket.connect(Unknown Source)
at java.net.Socket.connect(Unknown Source)
at sun.net.NetworkClient.doConnect(Unknown Source)
at sun.net.www.http.HttpClient.openServer(Unknown Source)
at sun.net.www.http.HttpClient.openServer(Unknown Source)
at sun.net.www.http.HttpClient.<init>(Unknown Source)
at sun.net.www.http.HttpClient.New(Unknown Source)
at sun.net.www.http.HttpClient.New(Unknown Source)
at sun.net.www.protocol.http.HttpURLConnection.getNewHttpClient(Unknown Source)
at sun.net.www.protocol.http.HttpURLConnection.plainConnect0(Unknown Source)
at sun.net.www.protocol.http.HttpURLConnection.plainConnect(Unknown Source)
at sun.net.www.protocol.http.HttpURLConnection.connect(Unknown Source)
at sun.net.www.protocol.http.HttpURLConnection.getInputStream0(Unknown Source)
at sun.net.www.protocol.http.HttpURLConnection.getInputStream(Unknown Source)
at org.apache.felix.bundlerepository.impl.FileUtil.openURL(FileUtil.java:209)
... 12 more
12:12:08.763 INFO com.affymetrix.igb.IGB - bundle com.lorainelab.protannot;8.5.4 installed

Need a review for the branch

https://bitbucket.org/kkorey/kkorey-igb/branch/IGBF-1108-App-Internet

Thanks

Would you test the installer on Mac and Linux?

Please use the installer from the latest master branch - build #78 or greater.

Update: Test using installer from built #80 or greater.

New pull request was merged. Previous solution failed due to bug in java library InetAddress.isReachable method. Custom method now implemented.

I tested on mac with build #83.

This is still not good.

If I do as the instructions say, I still see the original problem.

Open IGB.
See that there is no 23andMe plugin in the tools menu.
Open the app manager.
Disconnect the internet.
Install the 23andMe plugin.
Observe the the app manager button sift from "install" in green to "uninstall" in red (everything just as if it had installed).
Observe that the 23andMe plugin is still not available in the tools menu.

If I disconnect the internet before I open the app manager, then the app manager is blank.

I am moving this back to the to-do column.

TODO: Handle local repository installation.

I tested on mac again (more recent build) and it looks correct.
I was very sure I was on a recent enough build to have included this commit, but the results I see now are different, and the best explanation is that I just had an older build than I thought I did.
My apologies.

Made changes to handle local app installations.

Functional test

I wanted to start by seeing the state of master. But my local build of master does not show any available plugins, even though I have the bioviz repo and the local ProtAnnot folder set as active app repositories. This is odd.

This issue could not be tested because there was a new bug on master that affected the app manager.
That bug is described in IGBF-1366 Fix App Manager failure to load bundles when running in Development mode
The fix for that bug is associated with IGBF-1371 Remove obsolete dependency statements from pom.xml

Now that the fix has been merged into master, I can move forward with testing this issue.

I think the initial App-Internet change fixed the internet issue, so it was merged into master, it but introduced a bug such that IGB could not handle local repositories. We discovered this when testing changes to the ProtAnnot code base. The newest commit on the IGBF-1108 branch should address this.

Test on:

• 9.0.1 release,
• current master, and
• Kiran's branch https://bitbucket.org/kkorey/kkorey-igb/branch/IGBF-1108-App-Internet

Test for:

• Apps should not be shown as "installed" if the internet connection was dropped and the app actually did not install (see steps in the description).
• A local repository should be able to display and install apps.

git review:

• Single commit with appropriate message. good.

code review:

• Why is the isInternetReachable() method public? It looks like it is only meant to be called within this class, and should be private.

• Is testing the string for "http" the best way to tell if a resource is local? (do urls ever start with "www"? could a local source also start with "http"? ---I feel like the same package that has the HttpURLConnection class should also have some test method to see if a url is the right type of url ... ?) Or that we might have something analogous to the "if (dataProvider instanceof LocalDataProvider)" statement that we saw in the BookmarkUnibrowControlServlet class. Just some thoughts.

• It seems a little odd to enter the isInternetReachable method, and step around the local file case. Intuitively, I would have expected to enter the isInternetReachable only for non-local cases. But that doesn't really matter.

Functional review:

9.0.1

• An app that cannot possibly have installed (because it turned off my connection after opening the app manager) says it is installed. --Bad
• Local ProtAnnot displays and installs. --Good.

Master branch

• Ann app that cannot install does not. I click the install button and it flickers, but it never lies to me. It might be nice if there was a message in the console, but not necessary. The console just says "ERROR o.l.i.p.manager.BundleActionManager - bioviz.org" for each time that I try. --Good.
• Local ProtAnnot displays, but cannot install. Clicking "install" does nothing. --Bad

Branch IGBF-1108-App-Internet

• Ann app that cannot install does not. I click the install button and it flickers, but it never lies to me. It might be nice if there was a message in the console, but not necessary. The console just says "ERROR o.l.i.p.manager.BundleActionManager - bioviz.org" for each time that I try. --Good.
• The local ProtAnnot displays and installs. --Good.

As far as I can tell, this branch has the desired functionality. Good!

I tried to think of a test to see if I could break the "http" string test, but that turned into more research than I wanted to do. --Please add some comment to assure me that that is a good comprehensive test that covers all cases we might expect.

I'm assigning this back to Kiran to address these comments:

• assure me that testing for "http" in the string is the best test, and covers all cases.
• change isInternetReachable to be private (or explain why it is public)
• Ponder a moment on the third code comment and see if it warrants any changes.

After that, please pass to Sneha to do a second first level review (use mine as an example).
By extension, this code review should consider the two commits that were originally on branch IGBF-1108-App-Internet (use git > show history in IDE to see those changes).

I changed the method to private and also changed the third comment in the code.

I had made the method public keeping in mind of reusing it if need elsewhere.
About HTTP - As far as I know, all the URL's begin with HTTP ( even though it is www, it will first start with http:// and then www) or ftp:// (which is not relevant here).

Changes reviewed and it looks good!.

Assigning back to Kiran Korey.

Pull request merged into master.

Tested on 9.0.2 Environment. The apps do not install and no error is thrown.

Moving it to done column.

Just to add an observation, when we were testing it on MAC OS, there was no repository selected by default when we opened app manager. In case of windows and Linux, the repository "http://bioviz.org/igbserver" was selected by default. Is it an expected behaviour?

Please repeat test on Mac after resetting all preferences to defaults. It is possible that the repository was de-activated on a prior run of IGB on the same machine.