-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add a create/update/changeId/delete posthook (#193) #242
Changes from 1 commit
6d51dfe
2f2ecb6
68cd544
d9271d4
45d8a32
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,14 +44,15 @@ | |
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import java.lang.ProcessBuilder; | ||
import java.io.OutputStream; | ||
import java.io.IOException; | ||
import java.io.StringWriter; | ||
import java.io.PrintWriter; | ||
import org.lsc.utils.output.LdifLayout; | ||
import com.fasterxml.jackson.databind.ObjectMapper; // For encoding object to JSON | ||
import com.fasterxml.jackson.databind.ObjectWriter; | ||
|
||
|
||
|
||
/** | ||
* This object is managing posthook scripts | ||
*/ | ||
|
@@ -63,31 +64,56 @@ public class Hooks { | |
* | ||
* return nothing | ||
*/ | ||
public final static void postSyncHook(final String hook, final LscModifications lm) { | ||
public final static void postSyncHook(final String hook, final String outputFormat, final LscModifications lm) { | ||
|
||
if( hook != null && ! hook.equals("") ) | ||
{ | ||
// Compute json modifications | ||
String jsonModifications = null; | ||
|
||
String format = ""; | ||
if( outputFormat.equals("json") ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the parsing of the outputFormat should be done previously, here you should be able to manipulate only an Enum |
||
format = "json"; | ||
} | ||
else | ||
{ | ||
format = "ldif"; | ||
} | ||
|
||
// Compute json/ldif modifications | ||
String modifications = null; | ||
|
||
switch (lm.getOperation()) { | ||
case CREATE_OBJECT: | ||
jsonModifications = getJsonModifications(lm); | ||
callHook("create", hook, lm.getMainIdentifier(), jsonModifications); | ||
if( format.equals("json") ) { | ||
modifications = getJsonModifications(lm); | ||
} | ||
else { | ||
modifications = LdifLayout.format(lm); | ||
} | ||
callHook("create", hook, lm.getMainIdentifier(), format, modifications); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these lines can be easily factorized, and in this case easily integrated in the callHook method |
||
break; | ||
|
||
case UPDATE_OBJECT: | ||
jsonModifications = getJsonModifications(lm); | ||
callHook("update", hook, lm.getMainIdentifier(), jsonModifications); | ||
if( format.equals("json") ) { | ||
modifications = getJsonModifications(lm); | ||
} | ||
else { | ||
modifications = LdifLayout.format(lm); | ||
} | ||
callHook("update", hook, lm.getMainIdentifier(), format, modifications); | ||
break; | ||
|
||
case CHANGE_ID: | ||
jsonModifications = getJsonModifications(lm); | ||
callHook("changeId", hook, lm.getMainIdentifier(), jsonModifications); | ||
if( format.equals("json") ) { | ||
modifications = getJsonModifications(lm); | ||
} | ||
else { | ||
modifications = LdifLayout.format(lm); | ||
} | ||
callHook("changeId", hook, lm.getMainIdentifier(), format, modifications); | ||
break; | ||
|
||
case DELETE_OBJECT: | ||
callHook("delete", hook, lm.getMainIdentifier(), jsonModifications); | ||
callHook("delete", hook, lm.getMainIdentifier(), format, modifications); | ||
break; | ||
|
||
default: | ||
|
@@ -96,20 +122,32 @@ public final static void postSyncHook(final String hook, final LscModifications | |
} | ||
} | ||
|
||
/** | ||
* Method calling the hook | ||
* | ||
* return nothing | ||
*/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure such comments are really useful |
||
public final static void callHook( String operationType, | ||
String hook, | ||
String identifier, | ||
String jsonModifications) { | ||
String format, | ||
String modifications) { | ||
|
||
LOGGER.info("Calling {} posthook {} for {}", operationType, hook, identifier); | ||
LOGGER.info("Calling {} posthook {} with format {} for {}", operationType, hook, format, identifier); | ||
try { | ||
if( jsonModifications != null ) { | ||
if( modifications != null ) { | ||
Process p = new ProcessBuilder( | ||
hook, | ||
identifier, | ||
operationType, | ||
jsonModifications) | ||
operationType) | ||
.start(); | ||
|
||
// sends ldif modifications to stdin of hook script | ||
OutputStream stdin = p.getOutputStream(); | ||
stdin.write(modifications.getBytes()); | ||
stdin.write("\n".getBytes()); | ||
stdin.flush(); | ||
stdin.close(); | ||
} | ||
else { | ||
Process p = new ProcessBuilder( | ||
|
@@ -123,16 +161,16 @@ public final static void callHook( String operationType, | |
StringWriter sw = new StringWriter(); | ||
PrintWriter pw = new PrintWriter(sw); | ||
e.printStackTrace(pw); | ||
LOGGER.error("Error while calling {} posthook {} for {}: {}", | ||
operationType,hook, identifier, sw.toString()); | ||
LOGGER.error("Error while calling {} posthook {} with format {} for {}: {}", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you know that the LOGGER.error can directly take an exception as a parameter, and so log the stack trace without all this stuff? |
||
operationType, hook, format, identifier, sw.toString()); | ||
} | ||
} | ||
|
||
/** | ||
* Method computing modifications as json | ||
* | ||
* @return modifications in a json String | ||
*/ | ||
* Method computing modifications as json | ||
* | ||
* @return modifications in a json String | ||
*/ | ||
public final static String getJsonModifications(final LscModifications lm) { | ||
ObjectWriter ow = new ObjectMapper().writer().withDefaultPrettyPrinter(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nits (perf): it's recommended to declare your ObjectMapper as static for performace reasons. |
||
String json = ""; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -197,6 +197,13 @@ public String getCondition(LscModificationType operation) { | |
return result; | ||
} | ||
|
||
public String getPostHookOutputFormat() { | ||
if (conf.getHooks() == null || conf.getHooks().getOutputFormat() == null) { | ||
return ""; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As said earlier, the result of this method should be an enum. |
||
} | ||
return conf.getHooks().getOutputFormat(); | ||
} | ||
|
||
public String getCreatePostHook() { | ||
if (conf.getHooks() == null || conf.getHooks().getCreatePostHook() == null) { | ||
return ""; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: I usually prefer returning an Optional to indicate there is a value or not. But here I did not check what are the consequences, it can be a little more complicated to handle if you are not used to. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,6 +45,15 @@ | |
*/ | ||
package org.lsc; | ||
|
||
import com.fasterxml.jackson.databind.ObjectMapper; // For encoding object to JSON | ||
import com.fasterxml.jackson.databind.ObjectWriter; | ||
import com.fasterxml.jackson.databind.JsonNode; | ||
import com.fasterxml.jackson.core.JsonFactory; | ||
import com.fasterxml.jackson.core.JsonParseException; | ||
import com.fasterxml.jackson.core.JsonParser; | ||
import java.io.StringWriter; | ||
import java.io.PrintWriter; | ||
|
||
import static org.junit.Assert.assertEquals; | ||
import static org.junit.Assert.assertFalse; | ||
import static org.junit.Assert.assertNotNull; | ||
|
@@ -55,7 +64,6 @@ | |
import org.junit.Before; | ||
import org.junit.Test; | ||
import org.lsc.configuration.LscConfiguration; | ||
|
||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
|
@@ -81,29 +89,78 @@ public void setup() { | |
} | ||
|
||
@Test | ||
public final void testLdap2LdapHookSyncTest() throws Exception { | ||
public final void testLdap2LdapJSONHookSyncTest() throws Exception { | ||
|
||
// Declare the tasks to launch in the correct order | ||
List<String> sync_tasks = Arrays.asList("ldap2ldapJSONHookTestCreate", "ldap2ldapJSONHookTestUpdate"); | ||
List<String> clean_tasks = Arrays.asList("ldap2ldapJSONHookTestDelete"); | ||
|
||
// perform the sync | ||
launchSyncCleanTask(sync_tasks, false, true, false); | ||
launchSyncCleanTask(clean_tasks, false, false, true); | ||
|
||
// check the results of the synchronization | ||
reloadJndiConnections(); | ||
|
||
ObjectMapper mapperCreatedEntry = new ObjectMapper(); | ||
JsonNode expectedCreatedEntry = mapperCreatedEntry.readTree("[ { \"attributeName\" : \"objectClass\", \"values\" : [ \"inetOrgPerson\", \"person\", \"top\" ], \"operation\" : \"ADD_VALUES\" }, { \"attributeName\" : \"cn\", \"values\" : [ \"CN0001-hook\" ], \"operation\" : \"ADD_VALUES\" }, { \"attributeName\" : \"sn\", \"values\" : [ \"CN0001-hook\" ], \"operation\" : \"ADD_VALUES\" } ]"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you add a binary attribute to your test? It will allow to check for regression regarding the handling of binary attributes in hooks. |
||
|
||
checkJSONSyncResults("create", expectedCreatedEntry); | ||
|
||
ObjectMapper mapperUpdatedEntry = new ObjectMapper(); | ||
JsonNode expectedUpdatedEntry = mapperUpdatedEntry.readTree("[ { \"attributeName\" : \"description\", \"values\" : [ \"CN0001-hook\" ], \"operation\" : \"REPLACE_VALUES\" } ]"); | ||
|
||
checkJSONSyncResults("update", expectedUpdatedEntry); | ||
|
||
checkJSONSyncResults("delete", null); | ||
} | ||
|
||
@Test | ||
public final void testLdap2LdapLDIFHookSyncTest() throws Exception { | ||
|
||
// Declare the tasks to launch in the correct order | ||
List<String> sync_tasks = Arrays.asList("ldap2ldapHookTestCreate", "ldap2ldapHookTestUpdate"); | ||
List<String> clean_tasks = Arrays.asList("ldap2ldapHookTestDelete"); | ||
List<String> sync_tasks = Arrays.asList("ldap2ldapLDIFHookTestCreate", "ldap2ldapLDIFHookTestUpdate"); | ||
List<String> clean_tasks = Arrays.asList("ldap2ldapLDIFHookTestDelete"); | ||
|
||
// perform the sync | ||
launchSyncCleanTask(sync_tasks, false, true, false); | ||
launchSyncCleanTask(clean_tasks, false, false, true); | ||
|
||
// check the results of the synchronization | ||
reloadJndiConnections(); | ||
checkSyncResults(); | ||
|
||
List<String> expectedCreatedEntry = Arrays.asList( | ||
"dn: cn=CN0001-hook,ou=ldap2ldap2TestTaskDst,ou=Test Data,dc=lsc-project,dc=org", | ||
"changetype: add", | ||
"objectClass: inetOrgPerson", | ||
"objectClass: person", | ||
"objectClass: top", | ||
"cn: CN0001-hook", | ||
"sn: CN0001-hook" | ||
); | ||
|
||
checkLDIFSyncResults("create", expectedCreatedEntry); | ||
|
||
List<String> expectedUpdatedEntry = Arrays.asList( | ||
"dn: cn=CN0001-hook,ou=ldap2ldap2TestTaskDst,ou=Test Data,dc=lsc-project,dc=org", | ||
"changetype: modify", | ||
"replace: description", | ||
"description: CN0001-hook" | ||
); | ||
|
||
checkLDIFSyncResults("update", expectedUpdatedEntry); | ||
|
||
checkLDIFSyncResults("delete", new ArrayList<String>()); | ||
} | ||
|
||
/* | ||
* Read hook log file to check passed arguments | ||
* Read hook log file to check passed arguments and modification passed as input | ||
*/ | ||
private final void checkSyncResults() throws Exception { | ||
private final void checkLDIFSyncResults(String operation, List<String> expectedEntry) throws Exception { | ||
|
||
List<String> hookResults = new ArrayList<String>(); | ||
try { | ||
File hookFile = new File("hook.log"); | ||
File hookFile = new File("hook-ldif-" + operation + ".log"); | ||
Scanner hookReader = new Scanner(hookFile); | ||
|
||
while (hookReader.hasNextLine()) { | ||
|
@@ -113,16 +170,61 @@ private final void checkSyncResults() throws Exception { | |
hookReader.close(); | ||
hookFile.delete(); // remove hook log | ||
} catch (Exception e) { | ||
fail("Error while reading hook.log"); | ||
fail("Error while reading hook-ldif-" + operation + ".log"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you don't really need to try/catch in test, the test will fail anyway in case of unexpected exception. |
||
} | ||
assertEquals(hookResults.get(0), "cn=CN0001-hook,ou=ldap2ldap2TestTaskDst,ou=Test Data,dc=lsc-project,dc=org"); | ||
assertEquals(hookResults.get(1), "create"); | ||
assertTrue("Hook logs: ADD_VALUES not found in created entry", hookResults.get(2).matches("^.*ADD_VALUES.*$")); | ||
assertEquals(hookResults.get(3), "cn=CN0001-hook,ou=ldap2ldap2TestTaskDst,ou=Test Data,dc=lsc-project,dc=org"); | ||
assertEquals(hookResults.get(4), "update"); | ||
assertTrue("Hook logs: REPLACE_VALUES not found in updated entry", hookResults.get(5).matches("^.*REPLACE_VALUES.*$")); | ||
assertEquals(hookResults.get(6), "cn=CN0001-hook,ou=ldap2ldap2TestTaskDst,ou=Test Data,dc=lsc-project,dc=org"); | ||
assertEquals(hookResults.get(7), "delete"); | ||
assertEquals(hookResults.get(1), operation); | ||
|
||
if(operation != "delete") { | ||
// Make sure all attributes in expectedEntry are present in the hook file | ||
List<String> entry = new ArrayList(hookResults.subList(3, (hookResults.size()-1))); | ||
for (String attr : expectedEntry) { | ||
assertTrue("Attribute " + attr + " not found in " + operation + " entry " + entry.toString(), entry.contains(attr)); | ||
} | ||
} | ||
|
||
} | ||
|
||
/* | ||
* Read hook log file to check passed arguments and modification passed as input | ||
*/ | ||
private final void checkJSONSyncResults(String operation, JsonNode expectedEntry) throws Exception { | ||
|
||
List<String> hookResults = new ArrayList<String>(); | ||
try { | ||
File hookFile = new File("hook-json-" + operation + ".log"); | ||
Scanner hookReader = new Scanner(hookFile); | ||
|
||
while (hookReader.hasNextLine()) { | ||
String data = hookReader.nextLine(); | ||
hookResults.add(data); | ||
} | ||
hookReader.close(); | ||
hookFile.delete(); // remove hook log | ||
} catch (Exception e) { | ||
fail("Error while reading hook-json-" + operation + ".log"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. idem |
||
} | ||
assertEquals(hookResults.get(0), "cn=CN0001-hook,ou=ldap2ldap2TestTaskDst,ou=Test Data,dc=lsc-project,dc=org"); | ||
assertEquals(hookResults.get(1), operation); | ||
|
||
if(operation != "delete") { | ||
// Make sure all attributes in expectedEntry are present in the hook file | ||
String entry = String.join("", new ArrayList(hookResults.subList(2, hookResults.size()))); | ||
|
||
ObjectMapper mapper = new ObjectMapper(); | ||
JsonFactory factory = mapper.getJsonFactory(); | ||
JsonParser jp = factory.createJsonParser(entry); | ||
try { | ||
JsonNode hookOperation = mapper.readTree(jp); | ||
assertEquals(hookOperation, expectedEntry); | ||
} | ||
catch (Exception e) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. idem |
||
StringWriter sw = new StringWriter(); | ||
PrintWriter pw = new PrintWriter(sw); | ||
e.printStackTrace(pw); | ||
fail("Error while decoding operation in hook-json-" + operation + ".log as JSON:" + sw.toString()); | ||
} | ||
} | ||
|
||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: in general in Java we don't use static methods. You can construct the object in the constructor of the caller.