You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/04/24 08:59:34 UTC
[GitHub] [nifi] karthik-kadajji opened a new pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
karthik-kadajji opened a new pull request #4231:
URL: https://github.com/apache/nifi/pull/4231
Thank you for submitting a contribution to Apache NiFi.
Please provide a short description of the PR here:
#### Description of PR
_Enables X functionality; fixes bug NIFI-YYYY._
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [X ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [X ] Does your PR title start with **NIFI-XXXX** where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [X ] Has your PR been rebased against the latest commit within the target branch (typically `master`)?
- [ X] Is your initial contribution a single, squashed commit? _Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not `squash` or use `--force` when pushing to allow for clean monitoring of changes._
### For code changes:
- [ X] Have you ensured that the full suite of tests is executed via `mvn -Pcontrib-check clean install` at the root `nifi` folder?
- [ X] Have you written or updated unit tests to verify your changes?
- [ ] Have you verified that the full build is successful on both JDK 8 and JDK 11?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE` file, including the main `LICENSE` file under `nifi-assembly`?
- [ ] If applicable, have you updated the `NOTICE` file, including the main `NOTICE` file found under `nifi-assembly`?
- [ X] If adding new Properties, have you added `.displayName` in addition to .name (programmatic access) for each of the new properties?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] karthik-kadajji commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
karthik-kadajji commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r415619429
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/test/java/org/apache/nifi/processors/mongodb/GetMongoIT.java
##########
@@ -61,9 +64,9 @@
static {
CAL = Calendar.getInstance();
DOCUMENTS = Lists.newArrayList(
- new Document("_id", "doc_1").append("a", 1).append("b", 2).append("c", 3),
- new Document("_id", "doc_2").append("a", 1).append("b", 2).append("c", 4).append("date_field", CAL.getTime()),
- new Document("_id", "doc_3").append("a", 1).append("b", 3)
+ new Document("_id", "doc_1").append("a", 1).append("b", 2).append("c", 3),
Review comment:
Yes, it is due to earlier version mismatch, for continuous indentation it is 8 spaces in some places and 4 at some. However, I have indented as before.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] karthik-kadajji commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
karthik-kadajji commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r415618114
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/test/java/org/apache/nifi/processors/mongodb/GetMongoIT.java
##########
@@ -668,4 +671,53 @@ public void testSendEmpty() throws Exception {
MockFlowFile flowFile = flowFiles.get(0);
Assert.assertEquals(0, flowFile.getSize());
}
+
+
+ @Test
+ public void testReadUserPaswd() throws Exception {
+ final String username = "myuser";
+ final String password = "password";
+ mongoClient = new MongoClient(new MongoClientURI(MONGO_URI));
+ final MongoDatabase db = mongoClient.getDatabase(DB_NAME);
+ final BasicDBObject createUserCommand = new BasicDBObject("createUser", username).append("pwd", password).append("roles",
+ java.util.Collections.singletonList(new BasicDBObject("role", "dbOwner").append("db", DB_NAME)));
+
+ BasicDBObject getUsersInfoCommand = new BasicDBObject("usersInfo", new BasicDBObject("user", username).append("db", DB_NAME));
+ Document result = db.runCommand(getUsersInfoCommand);
+ BasicDBObject dropUserCommand = new BasicDBObject("dropUser", username);
+
+ ArrayList users = (ArrayList) result.get("users");
+ if (!users.isEmpty()) {
+ db.runCommand(dropUserCommand);
+ System.out.println("dropping user");
+ }
+ db.runCommand(createUserCommand);
+
+ //setting new property
+ runner.removeProperty(AbstractMongoProcessor.URI);
+ runner.setVariable("uri", "mongodb://localhost:27017/?authSource="+DB_NAME);
+ runner.setProperty(AbstractMongoProcessor.URI, "${uri}");
+ runner.setProperty(GetMongo.PASSWORD,password);
Review comment:
I added the spacing . Thank you
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] alopresto commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
alopresto commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r416248237
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/GetMongo.java
##########
@@ -253,7 +278,13 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
});
outgoingFlowFile = session.putAllAttributes(outgoingFlowFile, attributes);
- session.getProvenanceReporter().receive(outgoingFlowFile, getURI(context));
+ String uriPass = "";
+ if (context.getProperty(USER_NAME).getValue() != null) {
+ uriPass = "mongodb://" + context.getProperty(USER_NAME).getValue() + ":" + context.getProperty(PASSWORD).getValue() + "@" + getURI(context).substring(10);
Review comment:
I don't believe the URI is validated anywhere to ensure it starts with `mongodb://`, so arbitrarily starting the index at _10_ isn't guaranteed to produce the expected outcome.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] alopresto commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
alopresto commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r416245301
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/GetMongo.java
##########
@@ -85,6 +92,22 @@
.allowableValues(YES_PP, NO_PP)
.addValidator(Validator.VALID)
.build();
+ static final PropertyDescriptor USER_NAME = new PropertyDescriptor.Builder()
+ .name("User Name")
+ .displayName("username")
Review comment:
I think the `name` and `displayName` values are switched.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] alopresto commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
alopresto commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r416249400
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/GetMongo.java
##########
@@ -271,4 +302,59 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
}
}
+
+ @OnScheduled
+ public void createClient(ProcessContext context) throws IOException {
Review comment:
Most of this method appears to duplicate `AbstractMongoProcessor#createClient()`. I suggest the duplicated code should be refactored.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] github-actions[bot] closed pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #4231:
URL: https://github.com/apache/nifi/pull/4231
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] alopresto commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
alopresto commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r416248876
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/GetMongo.java
##########
@@ -253,7 +278,13 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
});
outgoingFlowFile = session.putAllAttributes(outgoingFlowFile, attributes);
- session.getProvenanceReporter().receive(outgoingFlowFile, getURI(context));
+ String uriPass = "";
+ if (context.getProperty(USER_NAME).getValue() != null) {
+ uriPass = "mongodb://" + context.getProperty(USER_NAME).getValue() + ":" + context.getProperty(PASSWORD).getValue() + "@" + getURI(context).substring(10);
Review comment:
Also, there is a note in the [MongoDB docs](https://docs.mongodb.com/manual/reference/connection-string/) regarding usernames and passwords that contain special characters:
> If the username or password includes the at sign @, colon :, slash /, or the percent sign % character, use [percent encoding](https://tools.ietf.org/html/rfc3986#section-2.1).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] HorizonNet commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
HorizonNet commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r415698289
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/test/java/org/apache/nifi/processors/mongodb/GetMongoIT.java
##########
@@ -668,4 +671,52 @@ public void testSendEmpty() throws Exception {
MockFlowFile flowFile = flowFiles.get(0);
Assert.assertEquals(0, flowFile.getSize());
}
+
+
+ @Test
+ public void testReadUserPaswd() throws Exception {
+ final String username = "myuser";
+ final String password = "password";
+ mongoClient = new MongoClient(new MongoClientURI(MONGO_URI));
+ final MongoDatabase db = mongoClient.getDatabase(DB_NAME);
+ final BasicDBObject createUserCommand = new BasicDBObject("createUser", username).append("pwd", password).append("roles",
+ java.util.Collections.singletonList(new BasicDBObject("role", "dbOwner").append("db", DB_NAME)));
+
+ BasicDBObject getUsersInfoCommand = new BasicDBObject("usersInfo", new BasicDBObject("user", username).append("db", DB_NAME));
+ Document result = db.runCommand(getUsersInfoCommand);
+ BasicDBObject dropUserCommand = new BasicDBObject("dropUser", username);
+
+ ArrayList users = (ArrayList) result.get("users");
+ if (!users.isEmpty()) {
+ db.runCommand(dropUserCommand);
+ }
+ db.runCommand(createUserCommand);
+
+ //setting new property
+ runner.removeProperty(AbstractMongoProcessor.URI);
+ runner.setVariable("uri", "mongodb://localhost:27017/?authSource=" + DB_NAME);
+ runner.setProperty(AbstractMongoProcessor.URI, "${uri}");
+ runner.setProperty(GetMongo.PASSWORD, password);
+ runner.setProperty(GetMongo.USER_NAME, username);
+
+ runner.setVariable("query", "{\"_id\": \"doc_2\"}");
+ runner.setProperty(GetMongo.QUERY, "${query}");
+ runner.setProperty(GetMongo.JSON_TYPE, GetMongo.JSON_STANDARD);
+ runner.run();
+ runner.assertAllFlowFilesTransferred(GetMongo.REL_SUCCESS, 1);
+
+ List<MockFlowFile> flowFiles = runner.getFlowFilesForRelationship(GetMongo.REL_SUCCESS);
+ byte[] raw = runner.getContentAsByteArray(flowFiles.get(0));
+ ObjectMapper mapper = new ObjectMapper();
+ Map<String, Object> parsed = mapper.readValue(raw, Map.class);
+ SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd");
+
+ // Drop the user which was created
+ db.runCommand(dropUserCommand);
+
+ Assert.assertTrue(parsed.get("date_field").getClass() == String.class);
+ Assert.assertTrue(((String) parsed.get("date_field")).startsWith(format.format(CAL.getTime())));
+
Review comment:
Remove empty line.
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/GetMongo.java
##########
@@ -253,7 +278,13 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
});
outgoingFlowFile = session.putAllAttributes(outgoingFlowFile, attributes);
- session.getProvenanceReporter().receive(outgoingFlowFile, getURI(context));
+ String uriPass="";
Review comment:
Please add spaces around the equals sing.
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/test/java/org/apache/nifi/processors/mongodb/GetMongoIT.java
##########
@@ -668,4 +671,52 @@ public void testSendEmpty() throws Exception {
MockFlowFile flowFile = flowFiles.get(0);
Assert.assertEquals(0, flowFile.getSize());
}
+
+
+ @Test
+ public void testReadUserPaswd() throws Exception {
+ final String username = "myuser";
+ final String password = "password";
+ mongoClient = new MongoClient(new MongoClientURI(MONGO_URI));
+ final MongoDatabase db = mongoClient.getDatabase(DB_NAME);
+ final BasicDBObject createUserCommand = new BasicDBObject("createUser", username).append("pwd", password).append("roles",
+ java.util.Collections.singletonList(new BasicDBObject("role", "dbOwner").append("db", DB_NAME)));
+
+ BasicDBObject getUsersInfoCommand = new BasicDBObject("usersInfo", new BasicDBObject("user", username).append("db", DB_NAME));
+ Document result = db.runCommand(getUsersInfoCommand);
+ BasicDBObject dropUserCommand = new BasicDBObject("dropUser", username);
+
+ ArrayList users = (ArrayList) result.get("users");
+ if (!users.isEmpty()) {
+ db.runCommand(dropUserCommand);
+ }
+ db.runCommand(createUserCommand);
+
+ //setting new property
+ runner.removeProperty(AbstractMongoProcessor.URI);
+ runner.setVariable("uri", "mongodb://localhost:27017/?authSource=" + DB_NAME);
+ runner.setProperty(AbstractMongoProcessor.URI, "${uri}");
+ runner.setProperty(GetMongo.PASSWORD, password);
+ runner.setProperty(GetMongo.USER_NAME, username);
+
+ runner.setVariable("query", "{\"_id\": \"doc_2\"}");
+ runner.setProperty(GetMongo.QUERY, "${query}");
+ runner.setProperty(GetMongo.JSON_TYPE, GetMongo.JSON_STANDARD);
+ runner.run();
+ runner.assertAllFlowFilesTransferred(GetMongo.REL_SUCCESS, 1);
+
+ List<MockFlowFile> flowFiles = runner.getFlowFilesForRelationship(GetMongo.REL_SUCCESS);
+ byte[] raw = runner.getContentAsByteArray(flowFiles.get(0));
+ ObjectMapper mapper = new ObjectMapper();
+ Map<String, Object> parsed = mapper.readValue(raw, Map.class);
+ SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd");
+
+ // Drop the user which was created
+ db.runCommand(dropUserCommand);
+
+ Assert.assertTrue(parsed.get("date_field").getClass() == String.class);
+ Assert.assertTrue(((String) parsed.get("date_field")).startsWith(format.format(CAL.getTime())));
+
+ }
+
Review comment:
Ditto.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] karthik-kadajji commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
karthik-kadajji commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r415621438
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/test/java/org/apache/nifi/processors/mongodb/GetMongoIT.java
##########
@@ -668,4 +671,53 @@ public void testSendEmpty() throws Exception {
MockFlowFile flowFile = flowFiles.get(0);
Assert.assertEquals(0, flowFile.getSize());
}
+
+
+ @Test
+ public void testReadUserPaswd() throws Exception {
+ final String username = "myuser";
+ final String password = "password";
+ mongoClient = new MongoClient(new MongoClientURI(MONGO_URI));
+ final MongoDatabase db = mongoClient.getDatabase(DB_NAME);
+ final BasicDBObject createUserCommand = new BasicDBObject("createUser", username).append("pwd", password).append("roles",
+ java.util.Collections.singletonList(new BasicDBObject("role", "dbOwner").append("db", DB_NAME)));
+
+ BasicDBObject getUsersInfoCommand = new BasicDBObject("usersInfo", new BasicDBObject("user", username).append("db", DB_NAME));
+ Document result = db.runCommand(getUsersInfoCommand);
+ BasicDBObject dropUserCommand = new BasicDBObject("dropUser", username);
+
+ ArrayList users = (ArrayList) result.get("users");
+ if (!users.isEmpty()) {
+ db.runCommand(dropUserCommand);
+ System.out.println("dropping user");
Review comment:
I have dropped the entire line, since it is a test case, didnt need the "dropping user" message or log
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] MikeThomsen commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
MikeThomsen commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r419174198
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/GetMongo.java
##########
@@ -271,4 +302,59 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
}
}
+
+ @OnScheduled
+ public void createClient(ProcessContext context) throws IOException {
Review comment:
@alopresto We started moving toward using a controller service to manage the client connection I think ~3 releases ago. The best place for these changes might be there, so we can start moving toward deprecating per-processor connection management altogether.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] alopresto commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
alopresto commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r416245565
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/GetMongo.java
##########
@@ -85,6 +92,22 @@
.allowableValues(YES_PP, NO_PP)
.addValidator(Validator.VALID)
.build();
+ static final PropertyDescriptor USER_NAME = new PropertyDescriptor.Builder()
+ .name("User Name")
+ .displayName("username")
+ .description("User for mongodb authentication")
+ .required(false)
+ .addValidator(Validator.VALID)
+ .sensitive(false)
+ .build();
+
+ static final PropertyDescriptor PASSWORD = new PropertyDescriptor.Builder()
+ .name("Password")
Review comment:
The `name` value should match the formatting of the _username_ field and a `displayName` should also be present.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] alopresto commented on pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
alopresto commented on pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#issuecomment-620314181
It looks like the `PutMongo` and `DeleteMongo` processors (as well as `GetMongoRecord`, `PutMongoRecord`, and `DeleteMongoRecord`) could also take advantage of these new property descriptors, and therefore the better location for them is in `AbstractMongoProcessor`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] karthik-kadajji commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
karthik-kadajji commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r416560474
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/GetMongo.java
##########
@@ -271,4 +302,59 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
}
}
+
+ @OnScheduled
+ public void createClient(ProcessContext context) throws IOException {
Review comment:
It is true I could have done the changes in AbstractMongoProcessor, but it would end up affecting the components of PutMongo. Also Since request was for just getMongo I did it this way.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] alopresto commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
alopresto commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r416740352
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/GetMongo.java
##########
@@ -253,7 +278,13 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
});
outgoingFlowFile = session.putAllAttributes(outgoingFlowFile, attributes);
- session.getProvenanceReporter().receive(outgoingFlowFile, getURI(context));
+ String uriPass = "";
+ if (context.getProperty(USER_NAME).getValue() != null) {
+ uriPass = "mongodb://" + context.getProperty(USER_NAME).getValue() + ":" + context.getProperty(PASSWORD).getValue() + "@" + getURI(context).substring(10);
Review comment:
This is not something I would expect the user to do manually. It should be done automatically by the processor.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] MikeThomsen commented on pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
MikeThomsen commented on pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#issuecomment-631800420
@karthik-kadajji It would be easier to apply this change to the Mongo controller service. If you look at all of the processors, you'll see that they have "client service" as an option. In the long run, I'm planning to start deprecating the current configuration options and have the Mongo processors use that because it matches our design patterns better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] github-actions[bot] commented on pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#issuecomment-826170838
We're marking this PR as stale due to lack of updates in the past few months. If after another couple of weeks the stale label has not been removed this PR will be closed. This stale marker and eventual auto close does not indicate a judgement of the PR just lack of reviewer bandwidth and helps us keep the PR queue more manageable. If you would like this PR re-opened you can do so and a committer can remove the stale tag. Or you can open a new PR. Try to help review other PRs to increase PR review bandwidth which in turn helps yours.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] HorizonNet commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
HorizonNet commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r414690223
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/test/java/org/apache/nifi/processors/mongodb/GetMongoIT.java
##########
@@ -61,9 +64,9 @@
static {
CAL = Calendar.getInstance();
DOCUMENTS = Lists.newArrayList(
- new Document("_id", "doc_1").append("a", 1).append("b", 2).append("c", 3),
- new Document("_id", "doc_2").append("a", 1).append("b", 2).append("c", 4).append("date_field", CAL.getTime()),
- new Document("_id", "doc_3").append("a", 1).append("b", 3)
+ new Document("_id", "doc_1").append("a", 1).append("b", 2).append("c", 3),
Review comment:
Was this done by IDE auto-indentation or because the earlier version did not match the general indentation? There are also similar places in the other classes.
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/test/java/org/apache/nifi/processors/mongodb/GetMongoIT.java
##########
@@ -668,4 +671,53 @@ public void testSendEmpty() throws Exception {
MockFlowFile flowFile = flowFiles.get(0);
Assert.assertEquals(0, flowFile.getSize());
}
+
+
+ @Test
+ public void testReadUserPaswd() throws Exception {
+ final String username = "myuser";
+ final String password = "password";
+ mongoClient = new MongoClient(new MongoClientURI(MONGO_URI));
+ final MongoDatabase db = mongoClient.getDatabase(DB_NAME);
+ final BasicDBObject createUserCommand = new BasicDBObject("createUser", username).append("pwd", password).append("roles",
+ java.util.Collections.singletonList(new BasicDBObject("role", "dbOwner").append("db", DB_NAME)));
+
+ BasicDBObject getUsersInfoCommand = new BasicDBObject("usersInfo", new BasicDBObject("user", username).append("db", DB_NAME));
+ Document result = db.runCommand(getUsersInfoCommand);
+ BasicDBObject dropUserCommand = new BasicDBObject("dropUser", username);
+
+ ArrayList users = (ArrayList) result.get("users");
+ if (!users.isEmpty()) {
+ db.runCommand(dropUserCommand);
+ System.out.println("dropping user");
Review comment:
Better would be to use a logger.
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/GetMongo.java
##########
@@ -253,7 +278,14 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
});
outgoingFlowFile = session.putAllAttributes(outgoingFlowFile, attributes);
- session.getProvenanceReporter().receive(outgoingFlowFile, getURI(context));
+ String uriPass="";
+ if (context.getProperty(USER_NAME).getValue() != null) {
+ uriPass = "mongodb://" + context.getProperty(USER_NAME).getValue() + ":" + context.getProperty(PASSWORD).getValue() + "@" + getURI(context).substring(10);
+
Review comment:
Remove this empty line.
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/test/java/org/apache/nifi/processors/mongodb/GetMongoIT.java
##########
@@ -668,4 +671,53 @@ public void testSendEmpty() throws Exception {
MockFlowFile flowFile = flowFiles.get(0);
Assert.assertEquals(0, flowFile.getSize());
}
+
+
+ @Test
+ public void testReadUserPaswd() throws Exception {
+ final String username = "myuser";
+ final String password = "password";
+ mongoClient = new MongoClient(new MongoClientURI(MONGO_URI));
+ final MongoDatabase db = mongoClient.getDatabase(DB_NAME);
+ final BasicDBObject createUserCommand = new BasicDBObject("createUser", username).append("pwd", password).append("roles",
+ java.util.Collections.singletonList(new BasicDBObject("role", "dbOwner").append("db", DB_NAME)));
+
+ BasicDBObject getUsersInfoCommand = new BasicDBObject("usersInfo", new BasicDBObject("user", username).append("db", DB_NAME));
+ Document result = db.runCommand(getUsersInfoCommand);
+ BasicDBObject dropUserCommand = new BasicDBObject("dropUser", username);
+
+ ArrayList users = (ArrayList) result.get("users");
+ if (!users.isEmpty()) {
+ db.runCommand(dropUserCommand);
+ System.out.println("dropping user");
+ }
+ db.runCommand(createUserCommand);
+
+ //setting new property
+ runner.removeProperty(AbstractMongoProcessor.URI);
+ runner.setVariable("uri", "mongodb://localhost:27017/?authSource="+DB_NAME);
+ runner.setProperty(AbstractMongoProcessor.URI, "${uri}");
+ runner.setProperty(GetMongo.PASSWORD,password);
Review comment:
To stay consistent, add a space between the arguments. There are also other similar places in this class.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] alopresto commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
alopresto commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r416744601
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/GetMongo.java
##########
@@ -271,4 +302,59 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
}
}
+
+ @OnScheduled
+ public void createClient(ProcessContext context) throws IOException {
Review comment:
I think all 6 of the MongoDB processors need this change if one does, so it should be made in a consistent manner. It appears the Jira is incomplete as it only indicates the `GetMongo` processor, but that's what this review process is intended to mitigate. Requirements gathering is notoriously difficult and a single user's request has to be balanced against the ongoing needs of the project as a whole. Any code changes introduced need to be well-understood, complete, tested, sustainable, and maintainable. I use the analogy from camping of "leave it better than you found it."
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] karthik-kadajji commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
karthik-kadajji commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r416561022
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/GetMongo.java
##########
@@ -85,6 +92,22 @@
.allowableValues(YES_PP, NO_PP)
.addValidator(Validator.VALID)
.build();
+ static final PropertyDescriptor USER_NAME = new PropertyDescriptor.Builder()
+ .name("User Name")
+ .displayName("username")
Review comment:
Thank you for pointing it out. I will add it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] karthik-kadajji commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
karthik-kadajji commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r416551860
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/GetMongo.java
##########
@@ -253,7 +278,13 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
});
outgoingFlowFile = session.putAllAttributes(outgoingFlowFile, attributes);
- session.getProvenanceReporter().receive(outgoingFlowFile, getURI(context));
+ String uriPass = "";
+ if (context.getProperty(USER_NAME).getValue() != null) {
+ uriPass = "mongodb://" + context.getProperty(USER_NAME).getValue() + ":" + context.getProperty(PASSWORD).getValue() + "@" + getURI(context).substring(10);
Review comment:
I will add in the validation for the starting 10 characters for the URI.
I did see the MongoDB doc, for percent encoding which would be better:
a) forcing the user to handle the percent encoding and printing it in the description as a note for user.
b) explicitly handling of it by the processor.
current implementation is goes the first way. But if you think It is better for processor to handle, I will look into it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] karthik-kadajji commented on a change in pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
karthik-kadajji commented on a change in pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#discussion_r416560474
##########
File path: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/GetMongo.java
##########
@@ -271,4 +302,59 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
}
}
+
+ @OnScheduled
+ public void createClient(ProcessContext context) throws IOException {
Review comment:
It is true I could have done the changes in AbstractMongoProcessor, but it would end up affecting the components of PutMongo, if the ontrigger method of it's not modified, there would be an issue. Also Since request was for just getMongo I did it this way.
Regardless, do we have to create a new ticket to change processors getMongo and Delete Mongo or can I do with this itself?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] github-actions[bot] commented on pull request #4231: NIFI-7037 Split off username and password fields for GetMongo processor
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #4231:
URL: https://github.com/apache/nifi/pull/4231#issuecomment-826170838
We're marking this PR as stale due to lack of updates in the past few months. If after another couple of weeks the stale label has not been removed this PR will be closed. This stale marker and eventual auto close does not indicate a judgement of the PR just lack of reviewer bandwidth and helps us keep the PR queue more manageable. If you would like this PR re-opened you can do so and a committer can remove the stale tag. Or you can open a new PR. Try to help review other PRs to increase PR review bandwidth which in turn helps yours.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org