From c9f56551f05150c732b304a10d926cc64deb95be Mon Sep 17 00:00:00 2001 From: Nitin Motgi Date: Sat, 2 May 2020 10:43:38 -0700 Subject: [PATCH 1/4] [CDAP-15407] Fixed handling of error message when user types in a wrong directive or there is a syntax error in the directive typed --- .../registry/UserDirectiveRegistry.java | 4 ++-- .../service/directive/DirectivesHandler.java | 17 +++++++++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/wrangler-core/src/main/java/io/cdap/wrangler/registry/UserDirectiveRegistry.java b/wrangler-core/src/main/java/io/cdap/wrangler/registry/UserDirectiveRegistry.java index 214e8565a..5fb940d5c 100644 --- a/wrangler-core/src/main/java/io/cdap/wrangler/registry/UserDirectiveRegistry.java +++ b/wrangler-core/src/main/java/io/cdap/wrangler/registry/UserDirectiveRegistry.java @@ -129,8 +129,8 @@ public DirectiveInfo get(String namespace, String name) throws DirectiveLoadExce } if (directive == null) { throw new DirectiveLoadException( - String.format("10-5 - Unable to load the user defined directive '%s'. " + - "Please check if the artifact containing UDD is still present.", name) + String.format("Directive '%s' not found. Check if directive is spelled correctly or if directive is " + + "user defined make sure artifact is uploaded", name) ); } DirectiveInfo directiveInfo = new DirectiveInfo(DirectiveInfo.Scope.USER, directive); diff --git a/wrangler-service/src/main/java/io/cdap/wrangler/service/directive/DirectivesHandler.java b/wrangler-service/src/main/java/io/cdap/wrangler/service/directive/DirectivesHandler.java index 2945d0fab..42cc41bf9 100644 --- a/wrangler-service/src/main/java/io/cdap/wrangler/service/directive/DirectivesHandler.java +++ b/wrangler-service/src/main/java/io/cdap/wrangler/service/directive/DirectivesHandler.java @@ -55,6 +55,7 @@ import io.cdap.wrangler.api.Row; import io.cdap.wrangler.api.TokenGroup; import io.cdap.wrangler.api.TransientStore; +import io.cdap.wrangler.api.parser.SyntaxError; import io.cdap.wrangler.api.parser.Token; import io.cdap.wrangler.api.parser.TokenType; import io.cdap.wrangler.datamodel.DataModelGlossary; @@ -431,7 +432,7 @@ public void upload(HttpServiceRequest request, HttpServiceResponder responder, // Extract content. byte[] content = handler.getContent(); if (content == null) { - throw new BadRequestException("Body not present, please post the file containing the " + throw new BadRequestException("Body is not present, upload file containing " + "records to be wrangled."); } @@ -629,7 +630,7 @@ public void execute(HttpServiceRequest request, HttpServiceResponder responder, if ((object.getClass().getMethod("toString").getDeclaringClass() != Object.class)) { value.put(fieldName, object.toString()); } else { - value.put(fieldName, "Non-displayable object"); + value.put(fieldName, "Non displayable object"); } } else { value.put(fieldName, null); @@ -663,6 +664,18 @@ private String addLoadablePragmaDirectives(String namespace, Request request) { // Compile the directive extracting the loadable plugins (a.k.a // Directives in this context). CompileStatus status = compiler.compile(new MigrateToV2(request.getRecipe().getDirectives()).migrate()); + if (!status.isSuccess()) { + StringBuilder eStr = new StringBuilder(); + Iterator errors = status.getErrors(); + while (errors.hasNext()) { + eStr.append(errors.next().getMessage()); + if (errors.hasNext()) { + eStr.append(","); + } + } + throw new DirectiveParseException(eStr.toString()); + } + RecipeSymbol symbols = status.getSymbols(); Iterator iterator = symbols.iterator(); List userDirectives = new ArrayList<>(); From 6fcc43c4ecb3f4504417fe59388a87d9659ea6a8 Mon Sep 17 00:00:00 2001 From: Nitin Motgi Date: Wed, 6 May 2020 08:21:06 -0700 Subject: [PATCH 2/4] Resolve PR comments --- .../java/io/cdap/wrangler/registry/UserDirectiveRegistry.java | 4 ++-- .../io/cdap/wrangler/service/directive/DirectivesHandler.java | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/wrangler-core/src/main/java/io/cdap/wrangler/registry/UserDirectiveRegistry.java b/wrangler-core/src/main/java/io/cdap/wrangler/registry/UserDirectiveRegistry.java index 5fb940d5c..3c6de1f43 100644 --- a/wrangler-core/src/main/java/io/cdap/wrangler/registry/UserDirectiveRegistry.java +++ b/wrangler-core/src/main/java/io/cdap/wrangler/registry/UserDirectiveRegistry.java @@ -129,8 +129,8 @@ public DirectiveInfo get(String namespace, String name) throws DirectiveLoadExce } if (directive == null) { throw new DirectiveLoadException( - String.format("Directive '%s' not found. Check if directive is spelled correctly or if directive is " + - "user defined make sure artifact is uploaded", name) + String.format("Directive '%s' not found. Check if directive is spelled corrected. if the directive is " + + "user defined, make sure artifact has been uploaded", name) ); } DirectiveInfo directiveInfo = new DirectiveInfo(DirectiveInfo.Scope.USER, directive); diff --git a/wrangler-service/src/main/java/io/cdap/wrangler/service/directive/DirectivesHandler.java b/wrangler-service/src/main/java/io/cdap/wrangler/service/directive/DirectivesHandler.java index 42cc41bf9..ec90ee195 100644 --- a/wrangler-service/src/main/java/io/cdap/wrangler/service/directive/DirectivesHandler.java +++ b/wrangler-service/src/main/java/io/cdap/wrangler/service/directive/DirectivesHandler.java @@ -1307,6 +1307,7 @@ private List executeDirectives(NamespacedId id, @Nullable Request user, String migrate = migrator.migrate(); RecipeParser recipe = new GrammarBasedParser(id.getNamespace().getName(), migrate, composite); recipe.initialize(new ConfigDirectiveContext(configStore.getConfig())); + try { executor.initialize(recipe, context); rows = executor.execute(sample.apply(rows)); @@ -1318,6 +1319,7 @@ private List executeDirectives(NamespacedId id, @Nullable Request user, .stream() .filter(ErrorRecordBase::isShownInWrangler) .collect(Collectors.toList()); + if (errors.size() > 0) { throw new ErrorRecordsException(errors); } From 9903516c87be94f33501c91fc2ada5206419b478 Mon Sep 17 00:00:00 2001 From: Nitin Motgi Date: Wed, 6 May 2020 11:52:21 -0700 Subject: [PATCH 3/4] Fix the testing error --- .../src/test/java/io/cdap/wrangler/utils/Json2SchemaTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wrangler-core/src/test/java/io/cdap/wrangler/utils/Json2SchemaTest.java b/wrangler-core/src/test/java/io/cdap/wrangler/utils/Json2SchemaTest.java index 76bd48931..54104ce80 100644 --- a/wrangler-core/src/test/java/io/cdap/wrangler/utils/Json2SchemaTest.java +++ b/wrangler-core/src/test/java/io/cdap/wrangler/utils/Json2SchemaTest.java @@ -102,7 +102,7 @@ public void testLogicalType() throws Exception { Json2Schema json2Schema = new Json2Schema(); Schema actual = json2Schema.toSchema("testRecord", testRow); - Schema expected = Schema.recordOf("expectedRecord", + Schema expected = Schema.recordOf("testRecord", Schema.Field.of("id", Schema.nullableOf(Schema.of(Schema.Type.INT))), Schema.Field.of("name", Schema.nullableOf(Schema.of(Schema.Type.STRING))), Schema.Field.of("date", Schema.nullableOf(Schema.of(Schema.LogicalType.DATE))), @@ -134,7 +134,7 @@ public void testArrayType() throws Exception { Json2Schema json2Schema = new Json2Schema(); Schema actual = json2Schema.toSchema("testRecord", testRow); - Schema expected = Schema.recordOf("expectedRecord", + Schema expected = Schema.recordOf("testRecord", Schema.Field.of("id", Schema.nullableOf(Schema.of(Schema.Type.INT))), Schema.Field.of("name", Schema.nullableOf(Schema.of(Schema.Type.STRING))), Schema.Field.of("date", Schema.nullableOf(Schema.of(Schema.LogicalType.DATE))), From a0c608247badf20023789c87d83462a1f2ec3b23 Mon Sep 17 00:00:00 2001 From: Nitin Motgi Date: Sun, 10 May 2020 13:55:42 -0700 Subject: [PATCH 4/4] Modified error message to have proper english --- .../java/io/cdap/wrangler/registry/UserDirectiveRegistry.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wrangler-core/src/main/java/io/cdap/wrangler/registry/UserDirectiveRegistry.java b/wrangler-core/src/main/java/io/cdap/wrangler/registry/UserDirectiveRegistry.java index 3c6de1f43..fab075021 100644 --- a/wrangler-core/src/main/java/io/cdap/wrangler/registry/UserDirectiveRegistry.java +++ b/wrangler-core/src/main/java/io/cdap/wrangler/registry/UserDirectiveRegistry.java @@ -129,8 +129,8 @@ public DirectiveInfo get(String namespace, String name) throws DirectiveLoadExce } if (directive == null) { throw new DirectiveLoadException( - String.format("Directive '%s' not found. Check if directive is spelled corrected. if the directive is " + - "user defined, make sure artifact has been uploaded", name) + String.format("Directive '%s' not found. Check if directive is spelled corrected. If the directive is " + + "user defined, make sure its artifact has been uploaded.", name) ); } DirectiveInfo directiveInfo = new DirectiveInfo(DirectiveInfo.Scope.USER, directive);