refactor (datafile manager): Update Headers type to only store at-most 1 value per header name (#5)

Summary:

Change `Headers` type to have at-most 1 value per header name. This is sufficient for the current requirements (which only use If-Modified-Since and Last-Modified), and simplifies the code. We can introduce a richer type if we need to deal with multiple values for headers.

Test plan:

Existing unit tests. This should be a pure refactor with no behavior change.

Author: mjc1283

packages/datafile-manager/src/http.ts

@@ -14,8 +14,15 @@
  * limitations under the License.
  */
 
+/**
+ * Headers is the interface that bridges between the abstract datafile manager and
+ * any Node-or-browser-specific http header types.
+ * It's simplified and can only store one value per header name.
+ * We can extend or replace this type if requirements change and we need
+ * to work with multiple values per header name.
+ */
 export interface Headers {
-  [header: string]: string | string[] | undefined;
+  [header: string]: string | undefined
 }
 
 export interface Response {

packages/datafile-manager/src/httpPollingDatafileManager.ts

@@ -262,15 +262,9 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana
 
   private trySavingLastModified(headers: Headers): void {
     const lastModifiedHeader = headers['last-modified'] || headers['Last-Modified']
-    if (typeof lastModifiedHeader === 'string') {
+    if (typeof lastModifiedHeader !== 'undefined') {
       this.lastResponseLastModified = lastModifiedHeader
       logger.debug('Saved last modified header value from response: %s', this.lastResponseLastModified)
-    } else if (typeof lastModifiedHeader === 'undefined') {
-    } else { // array
-      if (lastModifiedHeader.length === 1) {
-        this.lastResponseLastModified = lastModifiedHeader[0]
-        logger.debug('Saved last modified header value from response: %s', this.lastResponseLastModified)
-      }
     }
   }
 }

packages/datafile-manager/src/nodeRequest.ts

@@ -30,6 +30,35 @@ function getRequestOptionsFromUrl(url: url.UrlWithStringQuery): http.RequestOpti
   }
 }
 
+/**
+ * Convert incomingMessage.headers (which has type http.IncomingHttpHeaders) into our Headers type defined in src/http.ts.
+ *
+ * Our Headers type is simplified and can't represent mutliple values for the same header name.
+ *
+ * We don't currently need multiple values support, and the consumer code becomes simpler if it can assume at-most 1 value
+ * per header name.
+ *
+ */
+function createHeadersFromNodeIncomingMessage(
+  incomingMessage: http.IncomingMessage,
+): Headers {
+  const headers: Headers = {}
+  Object.keys(incomingMessage.headers).forEach(headerName => {
+    const headerValue = incomingMessage.headers[headerName]
+    if (typeof headerValue === 'string') {
+      headers[headerName] = headerValue
+    } else if (typeof headerValue === 'undefined') {
+    } else {
+      // array
+      if (headerValue.length > 0) {
+        // We don't care about multiple values - just take the first one
+        headers[headerName] = headerValue[0]
+      }
+    }
+  })
+  return headers
+}
+
 function getResponseFromRequest(request: http.ClientRequest): Promise<Response> {
   // TODO: When we drop support for Node 6, consider using util.promisify instead of
   // constructing own Promise
@@ -56,7 +85,7 @@ function getResponseFromRequest(request: http.ClientRequest): Promise<Response>
         resolve({
           statusCode: incomingMessage.statusCode,
           body: responseData,
-          headers: incomingMessage.headers,
+          headers: createHeadersFromNodeIncomingMessage(incomingMessage),
         })
       })
     })