Some more INFRA fixups (Issue #47)

- Fetch-Document silently returned an empty response if the requesting output device was not registered with the printer.
- Fetch-Document did not log an error if the document file has disappeared.
- The initial sequence number for subscriptions was not initialized.
- When a printer was added the printer-created and system-config-changed events were sent in one call, which doesn't work.
- Events would be added to a canceled subscription.
- The testpappl program did not re-add the output device to the infra printer in the register callback.

pappl/job-ipp.c

@@ -796,6 +796,8 @@ find_document_no_lock(
     return (NULL);
   }
 
+  _PAPPL_DEBUG("find_document_no_lock: job=%d, document[%d]={filename=\"%s\", format=\"%s\", state=%d}\n", client->job->job_id, doc_number, client->job->documents[doc_number - 1].filename, client->job->documents[doc_number - 1].format, client->job->documents[doc_number - 1].state);
+
   return (client->job->documents + doc_number - 1);
 }
 
@@ -1122,6 +1124,8 @@ ipp_fetch_document(
   _pappl_odevice_t	*od;		// Output device
 
 
+  _PAPPL_DEBUG("ipp_fetch_document(client=%p(%d), job=%p(%d))\n", client, client->number, client->job, client->job ? client->job->job_id : 0);
+
   // Authorize access...
   if (!_papplPrinterIsAuthorized(client))
     return;
@@ -1130,7 +1134,11 @@ ipp_fetch_document(
   _papplRWLockRead(printer);
   cupsRWLockWrite(&printer->output_rwlock);
 
-  if ((od = _papplClientFindDeviceNoLock(client)) != NULL)
+  if ((od = _papplClientFindDeviceNoLock(client)) == NULL)
+  {
+    papplClientRespondIPP(client, IPP_STATUS_ERROR_NOT_FOUND, "Output device not registered with printer.");
+  }
+  else
   {
     pappl_job_t		*job = client->job;
 					// Job
@@ -1141,6 +1149,8 @@ ipp_fetch_document(
 					// "compression" value to use
     //const char		*format;	// "document-format" value to use
 
+    _PAPPL_DEBUG("ipp_fetch_document: od=%p\n", od);
+
     if ((compressions = ippFindAttribute(client->request, "compression-accepted", IPP_TAG_ZERO)) != NULL)
     {
       if (ippGetGroupTag(compressions) != IPP_TAG_OPERATION || ippGetValueTag(compressions) != IPP_TAG_KEYWORD || (!ippContainsString(compressions, "none") && !ippContainsString(compressions, "gzip")))
@@ -1154,6 +1164,8 @@ ipp_fetch_document(
       }
     }
 
+    _PAPPL_DEBUG("ipp_fetch_document: compression='%s'\n", compression);
+
     if ((formats = ippFindAttribute(client->request, "document-format-accepted", IPP_TAG_ZERO)) != NULL)
     {
       if (ippGetGroupTag(formats) != IPP_TAG_OPERATION || ippGetValueTag(formats) != IPP_TAG_MIMETYPE)
@@ -1184,6 +1196,8 @@ ipp_fetch_document(
     }
     else if ((doc = find_document_no_lock(client)) != NULL)
     {
+      _PAPPL_DEBUG("ipp_fetch_document: doc={filename=\"%s\", format=\"%s\", state=%d}\n", doc->filename, doc->format, doc->state);
+
       if (doc->state >= IPP_DSTATE_CANCELED)
       {
 	// Document is in a terminating state...
@@ -1197,6 +1211,8 @@ ipp_fetch_document(
       else
       {
         // Send document to client...
+        _PAPPL_DEBUG("ipp_fetch_document: Sending document to client.\n");
+
         papplClientRespondIPP(client, IPP_STATUS_OK, /*message*/NULL);
         ippAddString(client->response, IPP_TAG_OPERATION, IPP_TAG_MIMETYPE, "document-format", /*language*/NULL, doc->format);
         ippAddString(client->response, IPP_TAG_OPERATION, IPP_TAG_KEYWORD, "compression", /*language*/NULL, compression);
@@ -1215,6 +1231,8 @@ ipp_fetch_document(
 
 	  if ((fd = open(doc->filename, O_RDONLY | O_BINARY)) >= 0)
 	  {
+	    _PAPPL_DEBUG("ipp_fetch_document: open(\"%s\")=%d\n", doc->filename, fd);
+
 	    if (!strcmp(compression, "gzip"))
 	      httpSetField(client->http, HTTP_FIELD_CONTENT_ENCODING, "gzip");
 
@@ -1230,6 +1248,10 @@ ipp_fetch_document(
 	  // Send a 0-length chunk...
 	  (void)httpWrite(client->http, "", 0);
 	}
+	else
+	{
+	  papplLogClient(client, PAPPL_LOGLEVEL_ERROR, "Unable to return document file '%s': %s", doc->filename, strerror(errno));
+	}
       }
     }
 

pappl/job.c

@@ -644,6 +644,8 @@ _papplJobRemoveFiles(pappl_job_t *job)	// I - Job
   char		filename[1024];		// Document attributes file
 
 
+  _PAPPL_DEBUG("**** _papplJobRemoveFiles(job=%p(%d)) ****\n", job, job->job_id);
+
   for (doc_number = 1, doc = job->documents; doc_number <= job->num_documents; doc_number ++, doc ++)
   {
     // Only remove the file if it is in spool or temporary directory...
@@ -1074,7 +1076,7 @@ _papplPrinterCheckJobsNoLock(
       _papplRWUnlock(job);
     }
 
-    if (job->state == IPP_JSTATE_PENDING)
+    if (job->state == IPP_JSTATE_PENDING && !(job->state_reasons & PAPPL_JREASON_JOB_FETCHABLE))
     {
       cups_thread_t	t;		// Thread
 

pappl/subscription.c

@@ -1,7 +1,7 @@
 //
 // System event subscription functions for the Printer Application Framework
 //
-// Copyright © 2022-2024 by Michael R Sweet.
+// Copyright © 2022-2025 by Michael R Sweet.
 //
 // Licensed under Apache License v2.0.  See the file "LICENSE" for more
 // information.
@@ -142,7 +142,8 @@ papplSubscriptionCreate(
   if (data && datalen > 0)
     ippAddOctetString(sub->attrs, IPP_TAG_SUBSCRIPTION, "notify-user-data", data, (size_t)datalen);
 
-  sub->events = cupsArrayNew(NULL, NULL, NULL, 0, NULL, (cups_afree_cb_t)ippDelete);
+  sub->first_sequence = 1;
+  sub->events         = cupsArrayNew(NULL, NULL, NULL, 0, NULL, (cups_afree_cb_t)ippDelete);
 
   return (sub);
 }

pappl/system-printer.c

@@ -46,7 +46,8 @@ _papplSystemAddPrinter(
   _papplRWUnlock(system);
 
   _papplSystemConfigChanged(system);
-  papplSystemAddEvent(system, printer, NULL, PAPPL_EVENT_PRINTER_CREATED | PAPPL_EVENT_SYSTEM_CONFIG_CHANGED, NULL);
+  papplSystemAddEvent(system, printer, NULL, PAPPL_EVENT_PRINTER_CREATED, NULL);
+  papplSystemAddEvent(system, printer, NULL, PAPPL_EVENT_SYSTEM_CONFIG_CHANGED, NULL);
 }
 
 

pappl/system-subscription.c

@@ -97,6 +97,8 @@ _papplSystemAddEventNoLockv(
   // Loop through all of the subscriptions and deliver any events...
   _papplRWLockRead(system);
 
+  _PAPPL_DEBUG("_papplSystemAddEventNoLockv(system=%p, printer=%p(%s), job=%p(%d), event=%x(%s), message=\"%s\", ap=%p)\n", system, printer, printer ? printer->name : "", job, job ? job->job_id : 0, event, _papplSubscriptionEventString(event), message, ap);
+
   if (system->systemui_cb && system->systemui_data)
     (system->systemui_cb)(system, printer, job, event, system->systemui_data);
 
@@ -105,6 +107,9 @@ _papplSystemAddEventNoLockv(
 
   for (sub = (pappl_subscription_t *)cupsArrayGetFirst(system->subscriptions); sub; sub = (pappl_subscription_t *)cupsArrayGetNext(system->subscriptions))
   {
+    if (sub->is_canceled)
+      continue;
+
     if ((sub->mask & event) && (!sub->job || job == sub->job) && (!sub->printer || printer == sub->printer))
     {
       _papplRWLockWrite(sub);
@@ -164,6 +169,8 @@ _papplSystemAddEventNoLockv(
 	sub->first_sequence ++;
       }
 
+      _PAPPL_DEBUG("_papplSystemAddEventNoLockv: Added event to sub=%p(%d). first=%d, last=%d\n", sub, sub->subscription_id, sub->first_sequence, sub->last_sequence);
+
       _papplRWUnlock(sub);
 
       cupsCondBroadcast(&system->subscription_cond);
@@ -243,7 +250,10 @@ _papplSystemCleanSubscriptions(
 
   // Now clean up the expired subscriptions...
   for (sub = (pappl_subscription_t *)cupsArrayGetFirst(expired); sub; sub = (pappl_subscription_t *)cupsArrayGetNext(expired))
+  {
+    _PAPPL_DEBUG("Deleting subscription %d\n", sub->subscription_id);
     _papplSubscriptionDelete(sub);
+  }
 
   cupsArrayDelete(expired);
 }

testsuite/testpappl.c

@@ -1049,7 +1049,9 @@ infra_register_cb(
   /* Look for a printer using this device */
   if ((printer = papplSystemFindInfraPrinter(papplClientGetSystem(client), device_uuid)) != NULL)
   {
-    /* Return existing printer */
+    /* (Re)add the device and return existing printer */
+    papplPrinterAddInfraDevice(printer, device_uuid);
+
     return (printer);
   }
   else if (papplPrinterIsInfra(requested_printer))