chore(git): Merge branch 'master' into fixing-sas9-tests

fix(browser): only import file I/O functions when running in Node.js environments

PULL_REQUEST_TEMPLATE.md

@@ -12,9 +12,9 @@ What code changes have been made to achieve the intent.
 
 ## Checks
 
-No PR (that involves a non-trivial code change) should be merged, unless all four of the items below are confirmed!  If an urgent fix is needed - use a tar file.
+No PR (that involves a non-trivial code change) should be merged, unless all items below are confirmed!  If an urgent fix is needed - use a tar file.
+
 
-- [ ] Code is formatted correctly (`npm run lint:fix`).
-- [ ] All unit tests are passing (`npm test`).
 - [ ] All `sasjs-cli` unit tests are passing (`npm test`).
 - [ ] All `sasjs-tests` are passing (instructions available [here](https://github.com/sasjs/adapter/blob/master/sasjs-tests/README.md)).
+- [ ] [Data Controller](https://datacontroller.io) builds and is functional on both SAS 9 and Viya

sasjs-tests/src/testSuites/Basic.ts

@@ -47,7 +47,9 @@ export const basicTests = (
         'Should fail on first attempt and should log the user in on second attempt',
       test: async () => {
         await adapter.logOut()
+        await sleep(1000)
         await adapter.logIn('invalid', 'invalid')
+        await sleep(1000)
         return adapter.logIn(userName, password)
       },
       assertion: (response: any) =>
@@ -151,6 +153,9 @@ export const basicTests = (
       description:
         'Should complete successful request with extra attributes present in response',
       test: async () => {
+        if (adapter.getSasjsConfig().serverType !== 'SASVIYA')
+          return Promise.resolve('skip')
+
         const config = {
           useComputeApi: false
         }
@@ -165,9 +170,15 @@ export const basicTests = (
         )
       },
       assertion: (response: any) => {
+        if (response === 'skip') return true
+
         const responseKeys: any = Object.keys(response)
         return responseKeys.includes('file') && responseKeys.includes('data')
       }
     }
   ]
 })
+
+const sleep = (ms: number) => {
+  return new Promise((resolve) => setTimeout(resolve, ms))
+}

src/api/viya/getFileStream.ts

@@ -0,0 +1,17 @@
+import { isFolder } from '@sasjs/utils/file'
+import { generateTimestamp } from '@sasjs/utils/time'
+import { Job } from '../../types'
+
+export const getFileStream = async (job: Job, filePath?: string) => {
+  const { createWriteStream } = require('@sasjs/utils/file')
+  const logPath = filePath || process.cwd()
+  const isFolderPath = await isFolder(logPath)
+  if (isFolderPath) {
+    const logFileName = `${job.name || 'job'}-${generateTimestamp()}.log`
+    const path = require('path')
+    const logFilePath = path.join(filePath || process.cwd(), logFileName)
+    return await createWriteStream(logFilePath)
+  } else {
+    return await createWriteStream(logPath)
+  }
+}

src/api/viya/pollJobState.ts

@@ -3,11 +3,8 @@ import { Job, PollOptions } from '../..'
 import { getTokens } from '../../auth/getTokens'
 import { RequestClient } from '../../request/RequestClient'
 import { JobStatePollError } from '../../types/errors'
-import { generateTimestamp } from '@sasjs/utils/time'
+import { Link, WriteStream } from '../../types'
-import { saveLog } from './saveLog'
+import { isNode } from '../../utils'
-import { createWriteStream, isFolder } from '@sasjs/utils/file'
-import { WriteStream } from 'fs'
-import { Link } from '../../types'
 
 export async function pollJobState(
   requestClient: RequestClient,
@@ -55,21 +52,9 @@ export async function pollJobState(
   }
 
   let logFileStream
-  if (pollOptions.streamLog) {
+  if (pollOptions.streamLog && isNode()) {
-    const logPath = pollOptions?.logFolderPath || process.cwd()
+    const { getFileStream } = require('./getFileStream')
-    const isFolderPath = await isFolder(logPath)
+    logFileStream = await getFileStream(postedJob, pollOptions.logFolderPath)
-    if (isFolderPath) {
-      const logFileName = `${
-        postedJob.name || 'job'
-      }-${generateTimestamp()}.log`
-      const logFilePath = `${
-        pollOptions?.logFolderPath || process.cwd()
-      }/${logFileName}`
-
-      logFileStream = await createWriteStream(logFilePath)
-    } else {
-      logFileStream = await createWriteStream(logPath)
-    }
   }
 
   // Poll up to the first 100 times with the specified poll interval
@@ -230,14 +215,17 @@ const doPoll = async (
 
       const endLogLine = job.logStatistics?.lineCount ?? 1000000
 
-      await saveLog(
+      const { saveLog } = isNode() ? require('./saveLog') : { saveLog: null }
-        postedJob,
+      if (saveLog) {
-        requestClient,
+        await saveLog(
-        startLogLine,
+          postedJob,
-        endLogLine,
+          requestClient,
-        logStream,
+          startLogLine,
-        authConfig?.access_token
+          endLogLine,
-      )
+          logStream,
+          authConfig?.access_token
+        )
+      }
 
       startLogLine += endLogLine
     }

src/api/viya/saveLog.ts

@@ -1,7 +1,7 @@
 import { Job } from '../..'
 import { RequestClient } from '../../request/RequestClient'
 import { fetchLog } from '../../utils'
-import { WriteStream } from 'fs'
+import { WriteStream } from '../../types'
 import { writeStream } from './writeStream'
 
 /**

src/api/viya/spec/getFileStream.spec.ts

@@ -0,0 +1,41 @@
+import { Logger, LogLevel } from '@sasjs/utils/logger'
+import * as path from 'path'
+import * as fileModule from '@sasjs/utils/file'
+import { getFileStream } from '../getFileStream'
+import { mockJob } from './mockResponses'
+import { WriteStream } from '../../../types'
+
+describe('getFileStream', () => {
+  beforeEach(() => {
+    ;(process as any).logger = new Logger(LogLevel.Off)
+    setupMocks()
+  })
+  it('should use the given log path if it points to a file', async () => {
+    const { createWriteStream } = require('@sasjs/utils/file')
+
+    await getFileStream(mockJob, path.join(__dirname, 'test.log'))
+
+    expect(createWriteStream).toHaveBeenCalledWith(
+      path.join(__dirname, 'test.log')
+    )
+  })
+
+  it('should generate a log file path with a timestamp if it points to a folder', async () => {
+    const { createWriteStream } = require('@sasjs/utils/file')
+
+    await getFileStream(mockJob, __dirname)
+
+    expect(createWriteStream).not.toHaveBeenCalledWith(__dirname)
+    expect(createWriteStream).toHaveBeenCalledWith(
+      expect.stringContaining(path.join(__dirname, 'test job-20'))
+    )
+  })
+})
+
+const setupMocks = () => {
+  jest.restoreAllMocks()
+  jest.mock('@sasjs/utils/file/file')
+  jest
+    .spyOn(fileModule, 'createWriteStream')
+    .mockImplementation(() => Promise.resolve({} as unknown as WriteStream))
+}

src/api/viya/spec/pollJobState.spec.ts

@@ -1,11 +1,11 @@
 import { Logger, LogLevel } from '@sasjs/utils'
-import * as path from 'path'
-import * as fileModule from '@sasjs/utils/file'
 import { RequestClient } from '../../../request/RequestClient'
 import { mockAuthConfig, mockJob } from './mockResponses'
 import { pollJobState } from '../pollJobState'
 import * as getTokensModule from '../../../auth/getTokens'
 import * as saveLogModule from '../saveLog'
+import * as getFileStreamModule from '../getFileStream'
+import * as isNodeModule from '../../../utils/isNode'
 import { PollOptions } from '../../../types'
 import { WriteStream } from 'fs'
 
@@ -77,42 +77,43 @@ describe('pollJobState', () => {
 
   it('should attempt to fetch and save the log after each poll when streamLog is true', async () => {
     mockSimplePoll()
+    const { saveLog } = require('../saveLog')
 
     await pollJobState(requestClient, mockJob, false, mockAuthConfig, {
       ...defaultPollOptions,
       streamLog: true
     })
 
-    expect(saveLogModule.saveLog).toHaveBeenCalledTimes(2)
+    expect(saveLog).toHaveBeenCalledTimes(2)
   })
 
-  it('should use the given log path if it points to a file', async () => {
+  it('should create a write stream in Node.js environment when streamLog is true', async () => {
     mockSimplePoll()
+    const { getFileStream } = require('../getFileStream')
+    const { saveLog } = require('../saveLog')
 
     await pollJobState(requestClient, mockJob, false, mockAuthConfig, {
       ...defaultPollOptions,
-      streamLog: true,
+      streamLog: true
-      logFolderPath: path.join(__dirname, 'test.log')
     })
 
-    expect(fileModule.createWriteStream).toHaveBeenCalledWith(
+    expect(getFileStream).toHaveBeenCalled()
-      path.join(__dirname, 'test.log')
+    expect(saveLog).toHaveBeenCalledTimes(2)
-    )
   })
 
-  it('should generate a log file path with a timestamp if it points to a folder', async () => {
+  it('should not create a write stream in a non-Node.js environment', async () => {
     mockSimplePoll()
+    jest.spyOn(isNodeModule, 'isNode').mockImplementation(() => false)
+    const { saveLog } = require('../saveLog')
+    const { getFileStream } = require('../getFileStream')
 
     await pollJobState(requestClient, mockJob, false, mockAuthConfig, {
       ...defaultPollOptions,
-      streamLog: true,
+      streamLog: true
-      logFolderPath: path.join(__dirname)
     })
 
-    expect(fileModule.createWriteStream).not.toHaveBeenCalledWith(__dirname)
+    expect(getFileStream).not.toHaveBeenCalled()
-    expect(fileModule.createWriteStream).toHaveBeenCalledWith(
+    expect(saveLog).not.toHaveBeenCalled()
-      expect.stringContaining(__dirname + '/test job-20')
-    )
   })
 
   it('should not attempt to fetch and save the log after each poll when streamLog is false', async () => {
@@ -247,7 +248,8 @@ const setupMocks = () => {
   jest.mock('../../../request/RequestClient')
   jest.mock('../../../auth/getTokens')
   jest.mock('../saveLog')
-  jest.mock('@sasjs/utils/file')
+  jest.mock('../getFileStream')
+  jest.mock('../../../utils/isNode')
 
   jest
     .spyOn(requestClient, 'get')
@@ -261,8 +263,9 @@ const setupMocks = () => {
     .spyOn(saveLogModule, 'saveLog')
     .mockImplementation(() => Promise.resolve())
   jest
-    .spyOn(fileModule, 'createWriteStream')
+    .spyOn(getFileStreamModule, 'getFileStream')
     .mockImplementation(() => Promise.resolve({} as unknown as WriteStream))
+  jest.spyOn(isNodeModule, 'isNode').mockImplementation(() => true)
 }
 
 const mockSimplePoll = (runningCount = 2) => {
@@ -308,7 +311,7 @@ const mockLongPoll = () => {
       return Promise.resolve({ result: mockJob, etag: '', status: 200 })
     }
     return Promise.resolve({
-      result: count <= 101 ? 'running' : 'completed',
+      result: count <= 102 ? 'running' : 'completed',
       etag: '',
       status: 200
     })

src/api/viya/spec/saveLog.spec.ts

@@ -4,7 +4,7 @@ import * as fetchLogsModule from '../../../utils/fetchLogByChunks'
 import * as writeStreamModule from '../writeStream'
 import { saveLog } from '../saveLog'
 import { mockJob } from './mockResponses'
-import { WriteStream } from 'fs'
+import { WriteStream } from '../../../types'
 
 const requestClient = new (<jest.Mock<RequestClient>>RequestClient)()
 const stream = {} as unknown as WriteStream

src/api/viya/spec/writeStream.spec.ts

@@ -0,0 +1,25 @@
+import { WriteStream } from '../../../types'
+import { writeStream } from '../writeStream'
+import 'jest-extended'
+
+describe('writeStream', () => {
+  const stream: WriteStream = {
+    write: jest.fn(),
+    path: 'test'
+  }
+
+  it('should resolve when the stream is written successfully', async () => {
+    expect(writeStream(stream, 'test')).toResolve()
+
+    expect(stream.write).toHaveBeenCalledWith('test\n', expect.anything())
+  })
+
+  it('should reject when the write errors out', async () => {
+    jest
+      .spyOn(stream, 'write')
+      .mockImplementation((_, callback) => callback(new Error('Test Error')))
+    const error = await writeStream(stream, 'test').catch((e) => e)
+
+    expect(error.message).toEqual('Test Error')
+  })
+})

src/api/viya/writeStream.ts

@@ -1,11 +1,11 @@
-import { WriteStream } from 'fs'
+import { WriteStream } from '../../types'
 
 export const writeStream = async (
   stream: WriteStream,
   content: string
 ): Promise<void> => {
   return new Promise((resolve, reject) => {
-    stream.write(content + '\n\nnext chunk\n\n', (e) => {
+    stream.write(content + '\n', (e) => {
       if (e) {
         return reject(e)
       }

src/auth/getTokens.ts

@@ -1,9 +1,9 @@
 import {
-  AuthConfig,
   isAccessTokenExpiring,
   isRefreshTokenExpiring,
   hasTokenExpired
-} from '@sasjs/utils'
+} from '@sasjs/utils/auth'
+import { AuthConfig } from '@sasjs/utils/types'
 import { RequestClient } from '../request/RequestClient'
 import { refreshTokens } from './refreshTokens'
 

src/job-execution/WebJobExecutor.ts

@@ -107,20 +107,9 @@ export class WebJobExecutor extends BaseJobExecutor {
             this.appendRequest(res, sasJob, config.debug)
             resolve(jsonResponse)
           }
-          if (this.serverType === ServerType.Sas9 && config.debug) {
+          
-            const jsonResponse = parseWeboutResponse(res.result as string)
-            if (jsonResponse === '') {
-              throw new Error(
-                'Valid JSON could not be extracted from response.'
-              )
-            }
-
-            getValidJson(jsonResponse)
-            this.appendRequest(res, sasJob, config.debug)
-            resolve(res.result)
-          }
-          getValidJson(res.result as string)
           this.appendRequest(res, sasJob, config.debug)
+          getValidJson(res.result as string)
           resolve(res.result)
         })
         .catch(async (e: Error) => {

src/types/WriteStream.ts

@@ -0,0 +1,4 @@
+export interface WriteStream {
+  write: (content: string, callback: (err?: Error) => any) => void
+  path: string
+}

src/types/index.ts

@@ -11,3 +11,4 @@ export * from './SASjsRequest'
 export * from './Session'
 export * from './UploadFile'
 export * from './PollOptions'
+export * from './WriteStream'

src/utils/index.ts

@@ -1,6 +1,7 @@
 export * from './asyncForEach'
 export * from './compareTimestamps'
 export * from './convertToCsv'
+export * from './isNode'
 export * from './isRelativePath'
 export * from './isUri'
 export * from './isUrl'

src/utils/isNode.ts

@@ -0,0 +1,4 @@
+export const isNode = () =>
+  typeof process !== 'undefined' &&
+  process.versions != null &&
+  process.versions.node != null