Add new zss /jes end point for submitting jcl to jes #402

joepun76 · 2022-02-01T19:38:22Z

Signed-off-by: joepun76 [email protected]

Proposed changes

This PR addresses Issue: [Link to Github issue within https://github.com/zowe/zlux/issues if any]

This PR depends upon the following PRs:

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Change in a documentation
Refactor the code
Chore, repository cleanup, updates the dependencies.
Breaking change (fix or feature that would cause existing functionality to not work as expected)

PR Checklist

Please delete options that are not relevant.

If the changes in this PR are meant for the next release / mainline, this PR targets the "staging" branch.
My code follows the style guidelines of this project (see: Contributing guideline)
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
New and existing unit tests pass locally with my changes
video or image is included if visual changes are made
Relevant update to CHANGELOG.md
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works, or describe a test method below

Testing

Further comments

Add new end point /jes to submit jcl to jes (both jes2 and jes3 are support) with JOBID returned.

Signed-off-by: joepun76 <[email protected]>

ifakhrutdinov

Please have a look at my comments. FYI, I'm still reviewing jesService.c and jesrequestjson.c.

ifakhrutdinov · 2022-03-15T11:30:01Z

c/dsutils.c

@@ -0,0 +1,417 @@
+#ifdef METTLE
+/* HAS NOT BEEN COMPILED WITH METTLE BEFORE */


Do we need this comment? Perhaps #error Metal C is not supported or something similar would be better?

Replaced comment with #error Metal C is not supported

ifakhrutdinov · 2022-03-15T11:31:12Z

c/dsutils.c

+#ifdef METTLE
+/* HAS NOT BEEN COMPILED WITH METTLE BEFORE */
+#else
+#include <stdbool.h>


Do we need all these includes? For example, stdarg.h and bpxnet.h aren't needed.

Removed unnecessary includes

ifakhrutdinov · 2022-03-15T11:51:46Z

c/dsutils.c

@@ -0,0 +1,417 @@
+#ifdef METTLE


The file needs the copyright at the top and at the bottom (and any others without one).

Included copyright at the top and at the bottom

ifakhrutdinov · 2022-03-15T11:53:55Z

h/dsutils.h

+int getMaxRecordLength(char *dscb);
+
+int obtainDSCB1(const char *dsname, unsigned int dsnameLength,
+                       const char *volser, unsigned int volserLength,


Bad formatting.

Corrected formatting

ifakhrutdinov · 2022-03-15T11:56:59Z

h/dsutils.h

+#define INDEXED_DSCB 96
+
+static char defaultDatasetTypesAllowed[3] = {'A','D','X'};
+static char *defaultCSIFields[] ={ "NAME    ", "TYPE    ", "VOLSER  "};
+static int defaultCSIFieldCount = 3;
+
+const int DSCB_TRACE = FALSE;
+


These are only used in datasetjson.c, why do we need to expose them in a header?

They are used in dsutils.c as well

ifakhrutdinov · 2022-03-15T12:09:35Z

h/dsutils.h

+typedef struct Volser_tag {
+  char value[6]; /* space-padded */
+} Volser;
+


Can you provide doc for these new public functions? See examples in zowe-common-c/h/resmgr.h or other similar files.

Included comments similar to zowe-common-c/h/resmgr.h

ifakhrutdinov · 2022-03-15T12:21:51Z

h/dsutils.h

+  char value[6]; /* space-padded */
+} Volser;
+
+int getVolserForDataset(const DatasetName *dataset, Volser *volser);


Now that these are public, should we use a prefix for function names?

Used prefix "dsutils"

ifakhrutdinov · 2022-03-15T12:22:15Z

h/dsutils.h

+} Volser;
+
+int getVolserForDataset(const DatasetName *dataset, Volser *volser);
+char getRecordLengthType(char *dscb);


It's a good practise to use const for arguments you're not mutating, the definitions will need those too.

ifakhrutdinov · 2022-03-15T12:23:49Z

c/dsutils.c

+/*
+  Dataset only, no wildcards or pds members accepted - beware that this should be wrapped with authentication
+ */
+int getVolserForDataset(const DatasetName *dataset, Volser *volser) {


Is the content of this file the same as the one you removed from datasetjson.c?

Yes, it's the same except that I removed the 'static' in front of the functions.

ifakhrutdinov · 2022-03-15T12:48:33Z

h/jesrequestsjson.h

+#define __JESREQUESTSJSON__ 1
+
+
+#include "stcbase.h"


Why do you need all these headers here? Also, you can always use forward declaration for things like HttpResponse and void including unnecessary headers.

For example,

void respondWithSubmitDataset(struct HttpResponse_tag* response, const char* absolutePath, int jsonMode);

Removed this .h file and merged the changes to jesService.h. Only needed declarations were included to get .c file clean compiled

ifakhrutdinov · 2022-03-15T13:05:50Z

c/jesrequestsjson.c

+
+
+void responseWithJobDetails(HttpResponse* response, char* jobID, int jsonMode) {
+#ifdef __ZOWE_OS_ZOS


Can this be done once in the beginning of this source other than in every function?

Removed this .c file and merged the changes to jesService.c. Updated the code based on this review point

ifakhrutdinov · 2022-03-15T13:08:25Z

c/jesrequestsjson.c

+#define NATIVE_CODEPAGE CCSID_EBCDIC_1047
+#endif
+
+


What is the reason of having a separate source for this functionality as opposed to putting it into jesService.c?

All changes that are in jesrequestsjson.c were merged into jesService.c. Please check jesService.c for succeeding review points on jesrequestsjson.c

ifakhrutdinov · 2022-03-15T13:09:43Z

c/jesrequestsjson.c

+
+  jsonStart(jPrinter);
+
+  jsonAddUnterminatedString(jPrinter, "jobId", jobID, 8);


What if jobID is smaller than 8 bytes?

Updated the code to pass what is the exact bytes of the jobID

ifakhrutdinov · 2022-03-15T13:10:10Z

c/jesrequestsjson.c

+#else /* not __ZOWE_OS_ZOS */
+
+  /* Currently nothing else has "datasets" */
+  /* TBD: Is it really necessary to provide this empty array?


What array?

Deleted the comment

ifakhrutdinov · 2022-03-16T12:16:09Z

c/jesrequestsjson.c

+  }
+
+  int bufferSize = recordLength+1;
+  char buffer[bufferSize];


Just use DATA_STREAM_BUFFER_SIZE + 1, otherwise you may end up with VLAs.

Updated the code based on the comment

ifakhrutdinov · 2022-03-16T12:19:47Z

c/jesrequestsjson.c

+        // empty record
+        /* jsonAddString(jPrinter, NULL, ""); */
+      } else if (ferror(in)) {
+        zowelog(NULL, LOG_COMP_RESTDATASET, ZOWE_LOG_DEBUG,  "Error reading DSN=%s, rc=%d\n", filename, bytesRead);


Can you print the actual error code?

Added error number in the log

ifakhrutdinov · 2022-03-16T12:20:18Z

c/jesrequestsjson.c

+      }
+    }
+    else {
+      zowelog(NULL, LOG_COMP_RESTDATASET, ZOWE_LOG_DEBUG, "UPDATE DATASET: body was not JSON!\n");


Why do we use this component if it's not related to data sets?

Updated the correct component

ifakhrutdinov · 2022-03-16T12:21:43Z

c/jesrequestsjson.c

+
+  char *outACB = NULL;
+  RPLCommon *rpl = NULL;
+


Same issues as in responseWithSubmitJCLContents

Included checking of rc

ifakhrutdinov · 2022-03-16T12:22:54Z

c/jesService.c

+    setResponseStatus(response, 405, "Method Not Allowed");
+    addStringHeader(response, "Server", "jdmfws");
+    addStringHeader(response, "Transfer-Encoding", "chunked");
+    addStringHeader(response, "Allow", "GET, PUT, DELETE");


Do we allow GET and PUT?

Only PUT, updated the code

Signed-off-by: 1000TurquoisePogs <[email protected]>

Signed-off-by: Aditya Ranshinge <[email protected]>

Fix collision with debug ifdef

…with-zowelog

…intf-with-zowelog configure log component

Signed-off-by: Oscar Jr Celis <[email protected]>

Signed-off-by: ojelis <[email protected]>

ifakhrutdinov · 2022-06-07T19:48:55Z

h/dsutils.h

+ * @return lrecl return the length of a dataset 
+ * 0 this function encountered error 
+ */
+int dsutilsGetLreclOrRespondError(HttpResponse *response, const DatasetName *dsn, const char *ddPath);


The "respond" functions and the functions with http related code don't seem to belong here.

The name dsutils implies some generic data set utils, but the source contains http service speicific code . Did you intend that?

Updated dsutils and jesService.

ifakhrutdinov · 2022-06-07T19:48:55Z

h/dsutils.h

+ * @return lrecl return the length of a dataset 
+ * 0 this function encountered error 
+ */
+int dsutilsGetLreclOrRespondError(HttpResponse *response, const DatasetName *dsn, const char *ddPath);


The "respond" functions and the functions with http related code don't seem to belong here.

The name dsutils implies some generic data set utils, but the source contains http service speicific code . Did you intend that?

Updated dustils and jesService.

Signed-off-by: ojelis <[email protected]>

Signed-off-by: ojcelis <[email protected]>

initial

5e3aa0e

Signed-off-by: joepun76 <[email protected]>

joepun76 requested review from lchudinov, JoeNemo, 1000TurquoisePogs and ifakhrutdinov February 1, 2022 19:38

joepun76 changed the title ~~initial~~ Add new zss /jes end point for submitting jcl to jes Feb 1, 2022

ifakhrutdinov reviewed Mar 15, 2022

View reviewed changes

ifakhrutdinov requested changes Mar 16, 2022

View reviewed changes

1000TurquoisePogs and others added 6 commits March 21, 2022 17:10

Update pointer

1afc9d2

Signed-off-by: 1000TurquoisePogs <[email protected]>

configure log component

e76fe2b

Signed-off-by: Aditya Ranshinge <[email protected]>

Merge pull request zowe#432 from zowe/bugfix/json-debug-v1

386d49c

Fix collision with debug ifdef

Merge branch 'v1.x/staging' into users/aditya/feature/replace-printf-…

3a28c7e

…with-zowelog

Merge pull request zowe#436 from zowe/users/aditya/feature/replace-pr…

c67a66d

…intf-with-zowelog configure log component

Merge branch 'zowe:v1.x/staging' into jesSubmit

f28dffd

joepun76 assigned joepun76 and unassigned joepun76 Mar 30, 2022

ojcelis self-assigned this Apr 13, 2022

ojcelis added 2 commits May 9, 2022 15:56

Merge V2

9119986

Signed-off-by: Oscar Jr Celis <[email protected]>

Updated and addressed review points related to PR402

bbd29cc

Signed-off-by: ojelis <[email protected]>

JoeNemo changed the base branch from v1.x/staging to v2.x/staging May 25, 2022 15:08

ifakhrutdinov reviewed Jun 7, 2022

View reviewed changes

ojcelis and others added 7 commits June 15, 2022 12:15

Merge branch 'zowe:v2.x/staging' into jesSubmit

be6a01e

update submodule to the latest

930c92f

Signed-off-by: ojelis <[email protected]>

Addressed additional review points

cb38205

Signed-off-by: ojelis <[email protected]>

Merge v2.x/staging to jesSubmit branch

436d3ef

Signed-off-by: ojcelis <[email protected]>

fixed issue with 64 bit

4306e49

Signed-off-by: ojcelis <[email protected]>

update zowe-common-c to the latest

4b02de5

Signed-off-by: ojcelis <[email protected]>

Merge branch 'zowe:v2.x/staging' into jesSubmit

c966b6e

		@@ -0,0 +1,417 @@
		#ifdef METTLE
		/* HAS NOT BEEN COMPILED WITH METTLE BEFORE */



		void responseWithJobDetails(HttpResponse* response, char* jobID, int jsonMode) {
		#ifdef __ZOWE_OS_ZOS


		jsonStart(jPrinter);

		jsonAddUnterminatedString(jPrinter, "jobId", jobID, 8);

		#define __JESREQUESTSJSON__ 1


		#include "stcbase.h"

		#define NATIVE_CODEPAGE CCSID_EBCDIC_1047
		#endif

Add new zss /jes end point for submitting jcl to jes #402

Are you sure you want to change the base?

Add new zss /jes end point for submitting jcl to jes #402

Conversation

joepun76 commented Feb 1, 2022

Proposed changes

Type of change

PR Checklist

Testing

Further comments

ifakhrutdinov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ojcelis May 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ojcelis May 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ojcelis May 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ojcelis May 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ojcelis May 16, 2022 •

edited

Loading

ojcelis May 16, 2022 •

edited

Loading

ojcelis May 16, 2022 •

edited

Loading

ojcelis May 16, 2022 •

edited

Loading