Allow Apps to function when offline

See apps_server_local() in apps.manifest.inc for relevant code. For this to work, the local apps need to have relevant files (modulename.manifest plus screenshot and icon files) and need to designate these in an apps array in their .info file.

Having a site with no internet access while not ideal is, sadly, not uncommon. @populist and I were talking here in Munich and we think we have a way to handle this.

In order to work with apps that are local and the site cannot pull a manifest from the app server, each of the apps will need to be able to describe themselves to the app console. In order to achieve this, each app should have it's full manifest encoded into it's .info file. This will allow the module to inspect the contents of a module's .info file to determine if it is an app, and if so, what information it should display in the console. As part of this we will need to migrate the screenshots to be handled in much the same way a theme can handle the screenshots.

One issue to work out is how to handle screenshots from a local app (relative path to the module dir) and remote apps that are not downloaded to the site yet. Are those screenshot urls fully qualified and included directly into an img tag, or do we continue the process of downloading the screenshots locally.

As noted in #1, a lot of this was already sketched in in apps_server_local() so that's probably the place to start here. That function seems to have at least one typo.

      if($info->info['apps']['server'] = $server['name']) {

Presumably that should be == instead of =.

I wonder if it's necessary to have a full manifest in each main app module. Can we assume that, if an app is present, its dependencies will also be present? I believe we do make that assumption currently in at least a couple of places.

At install time and also when determining if an app is available we check for locally available apps. But in doing so I believe we're just checking for the presence of the main app module.

Working on a patch for this. Will be a few days but hope to post something before the end of the week

@sylus Sorry I could not get to this last week, time got away from me. We are working on getting a sprint together on Apps this Friday with a few folks so I plan to post a patch on Friday. Hopefully that works with your timeframe. If not I can post what I have (knowing it is rather incomplete) and seeing where you can take it over the next couple of days.

Ok, here is the first crack at the patch. It adds a check box to the App config screen in admin/config that enables offline mode. Then it will inspect each module locally to see if there is an app[] key (that contains most of the manifest data) in the modules .info file. Basically as it stands now all apps are likely not compatible with offline mode, but can be made so by adding stuff into the .info file. Have a look and ask any questions that come up.

@febbraro: thanks for tackling this!

Looks good. The one thing I think we need to think through a bit more the 'apps_offline_mode' variable. We definitely need this, but I'm thinking, if there are local apps, we need to use their manifests even if we're not in offline mode.

If a distro ships with some of its apps available locally, the app server may have later versions of the apps, and so be incompatible with the what's actually installed. In that case, we'll want the local manifest data to take precedence.

We may also have a combination of app servers, some with local apps and some only remote. Here too we want the local data even if we're not in offline mode.

So it looks like the logic should be:

• Check for local manifests.
• If in apps_offline_mode, or if local manifest data were found for this server, stop and return just that local manifest data.
• Otherwise, query the server for manifest data.

Does that sound right?

@nedjo I agree more thought was needed. In thinking a bit deeper about this it is also laying the groundwork for update notifications. If the app server has a new version why would it need to be marked incompatible if all dependencies are met? Seems to me more like

1. We always scan local dirs and build a catalog.
2. If in offline mode, we are done.
3. If in online mode, we compare local catalog with server catalog
4. If no newer version, done
5. If newer version, evaluate dependencies to see if an update is eligible or if the update is incompatible.

This can bring us one step closer in that it will have the ability to notify of updates (but not perform them yet)

@febbraro: Yes, ideally we would be able to consider using a newer version as available from the app server. But, until we have the ability to do updates, we can't. Pretty much by definition, a newer version of an app will include updates to one or more already-installed modules--even if this is just the app feature itself.

So, yes, once we have the ability to do updates, we'll be able to consider your #5. But meantime, I don't think we have any choice but to use the local version.

Where we may be able to evaluate dependencies is in the case where we find local apps for a server and there are additional apps in the server manifest that aren't locally installed. I was thinking that in this case we would need to skip the server-only apps and return only the local apps. This because it's likely the server-only apps will have later dependency requirements than our local versions. But, I suppose, we could compare dependencies and, if the server-only app has no dependencies that are later than locally-installed ones, offer it for download.

That said, likely that sort of complexity could wait for a follow-up patch.

Ok, long overdue, here is the next version of the offline patch that supports what @nedjo suggested in #15. By default, it processes the local apps and the remote apps with local apps taking precedence. In offline mode, no remote apps are queried. Additionally, it marks the apps as either 'local' = TRUE or 'remote' = TRUE and if local and a remote version exists it will include the a key 'remote manifest' that contains all the details of the remote version (for use later on down the line)

There was a stray dpm in the patch.

@fabsor Dang! Sorry about that :( Thanks for the re-roll. How did the patch work out for you? Looking to get this committed so I can clear a log jam of other things waiting on the apps.manifest.inc file.

@nedjo If you see this comment, do you have anything you'd like to add seeing as you had some feedback on a previous version? The patch has been out a while and in the effort of moving things forward if I dont hear anything I will likely just commit it and make issues for anything that shakes out as a result. There are some changes to how apps should be "packaged" (meaning things to add to the modules .info file that identifies it as an app, and things like that can be addressed in separate tickets. Anyway, just a friendly ping if you were interested in reviewing before i move forward (likely next week).

Thanks @febbraro for the revised patch. Looks good! Didn't test, just gave a quick read through. Here are some minor comments. Commiting soon sounds good. We'll need documentation with examples of how to add local apps data to .info files.

Do we need a UI setting for apps_offline_mode?

+++ b/apps.manifest.inc
@@ -5,32 +5,54 @@
+/**
+ * Return all local apps for the specified server.
+ *

Should clarify that apps with no specified server will be returned as well. E.g.: Return all apps that either have no server given or that are for the specified server.

+++ b/apps.manifest.inc
@@ -5,32 +5,54 @@
 function apps_server_local($server) {

Maybe make the $server argument optional, with NULL returning all apps with no server specified? Overall, local Apps data has the potential to make Apps servers less relevant, which IMO is a good thing--it facilitates purely drupal.org based apps, see #1733748: [meta] Enable drupal.org based apps.

Where in the UI will Apps with no specified server show up? If a site has multiple Apps servers registered, will apps with no specified server show up multiple times?

Do we want the reverse: scan all local apps, discover servers that way?

+++ b/apps.manifest.inc
@@ -130,79 +158,97 @@ function apps_client_id() {
   if ($cache = cache_get("apps_manifest_{$server['name']}")) {
     return $cache->data;
   }

Does this test need to come after we check apps_offline_mode, so that we're sure we're getting only local data?

+++ b/apps.manifest.inc
@@ -130,79 +158,97 @@ function apps_client_id() {
+    $msg = t("Manifest error from @server: @error", array('@server' => $server['title'], '@error' => $request->error));
+    watchdog("apps", $msg);

$msg shouldn't be localized before being passed to watchdog.

+++ b/apps.manifest.inc
@@ -130,79 +158,97 @@ function apps_client_id() {
+    $msg = t("Manifest JSON from @server not parsable", array('@server' => $server['title']));
+    watchdog("apps", $msg);

Ditto.

Hi,

I'm really interested in your patch - but unfortunately I seem not to know enough to apply it...

I used:

git apply apps-offline-1621474-19.patch

All I get is this:

error: patch failed: apps.manifest.inc:5
error: apps.manifest.inc: patch does not apply
error: patch failed: apps.pages.inc:235
error: apps.pages.inc: patch does not apply

Do you have a hint to get me started?
I believe that applying this patch will fix this issue as well.

Thanks a lot!
Kai

I've outlined an alternate approach in #1905718: Refactor to support .make files natively; deprecate the manifest format and Apps servers that could vastly simplify the code while enabling offline apps.

#24:
If that works out well, using makefiles would be clearly preferable. Manifests are not supported by Drush, are more generally not drupalish and constitute one (or more) additional places to check.

With this patch applied, update module is not really a dependency anymore however it is listed in the .info file as one. This means that with Apps installed you can not disable the update module - which if you are using purely local features and apps may be desired for a number of reasons, performance being one of those.

Patch in #19 has been applied to 7.x-1.x, commit id d3aabf7c2d3ed7387dab5a57763cad774b695131

This has already been committed, so in case this needs further work it should be done in new issues. Marking this as fixed.