GEODE-1993: postprocess region/key #276
Conversation
Update for new ServerStarterRule
|
A few things:
|
Changed test to initialize region without REST calls. PostProcess query endpoints Load the RestSecurityService the Spring way
Postprocess an adhoc query Fixed some spelling errors
Fixed issue with parametrized queries throwing an NPE Removed System.outs that were put in during analysis Fixed code formatting after merge from develop Fixed @return without comment
|
Don't accept this PR yet. This is just a checkin to preserve work. Addressed all of the issues listed above. Added test for adhoc queries. Still need a test for named queries. Also need to verify functions won't return any region data without being post-processed. Have not run precheckin yet, either. |
Support named queries
Post process function results
|
precheckin running |
|
Precheckin successful except for one flaky test, QueueCommandsDUnitTest.testCreateUpdatesSharedConfig which doesn't appear to be related to this change. |
| Region region = | ||
| serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create(REGION_NAME); | ||
| region.put("key1", | ||
| "{\"@type\":\"com.gemstone.gemfire.web.rest.domain.Order\",\"purchaseOrderNo\":1121,\"customerId\":1012,\"description\":\"Order for XYZ Corp\",\"orderDate\":\"02/10/2014\",\"deliveryDate\":\"02/20/2014\",\"contact\":\"Jelly Bean\",\"email\":\"jelly.bean@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":1,\"description\":\"Product-100\",\"quantity\":12,\"unitPrice\":5,\"totalPrice\":60}],\"totalPrice\":225}"); |
There was a problem hiding this comment.
You could create an Order object and put it as an java object instead of using json. This would be much readable.
There was a problem hiding this comment.
True. The tests don't use the object, so I'll remove it for clarity.
| assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response)); | ||
|
|
||
| String body = IOUtils.toString(response.getEntity().getContent(), "UTF-8"); | ||
| assertTrue(body.startsWith("\"key1User/" + REGION_NAME + "/key1/")); |
There was a problem hiding this comment.
Because PdxBasedCrudController.read(region, keys, boolean) returns the value as a quoted string. E.g., "key1User/AuthRegion/key1/key1Value"
There was a problem hiding this comment.
Please verify if without the postProcessor, what it is returning? Does it still contain the quotes? This might be because you are using SamplePostProcessor which turns any object into a String result. Consider use a different post processor, that is specific to your "customers" region data.
|
|
||
| @BeforeClass | ||
| public static void before() throws Exception { | ||
| serverStarter.startServer(); |
There was a problem hiding this comment.
I think I just made a change in the rule that you don't need to call startServer anymore if you are using is as a rule.
| HttpResponse response = | ||
| restClient.doPost("/queries?id=selectCustomer&q=" + URLEncoder.encode(namedQuery, "UTF-8"), | ||
| "dataReader", "1234567", ""); | ||
| assertEquals(201, getCode(response)); |
There was a problem hiding this comment.
is there anyway to crate the named query using java api instead of rest api?
There was a problem hiding this comment.
I wasn't able to insert it via the API. When I did, my call to list queries always came up empty and I got a 404 when attempting to run it.
| assertEquals(200, getCode(response)); | ||
| assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response)); | ||
|
|
||
| String customerJson = |
There was a problem hiding this comment.
use java api to put the data in the regions instead of using rest api.
There was a problem hiding this comment.
This breaks from the pattern RestAPIsQueryAndFEJUnitTest follows. But as you said in an earlier comment about the Orders, it will read cleaner.
| @@ -87,6 +89,12 @@ | |||
| Object postProcess(Object principal, String regionPath, Object key, Object value, | |||
| boolean valueIsSerialized); | |||
|
|
|||
| Collection<Object> postProcess(Query query, Collection<String> regionNames, | |||
| Collection<Object> postProcess(Query query, Collection<String> regionNames, | ||
| Collection<Object> results); | ||
|
|
||
| Collection<Object> postProcess(Object principal, Query query, Collection<String> regionNames, |
| } | ||
|
|
||
| @Override | ||
| public Collection<Object> postProcess(Object principal, Query query, |
| } | ||
|
|
||
| @Override | ||
| public Collection<Object> postProcess(Query query, Collection<String> regionNames, |
| @@ -50,6 +52,11 @@ | |||
|
|
|||
| protected static final String REST_API_VERSION = "/v1"; | |||
|
|
|||
| public BaseControllerAdvice(final RestSecurityService securityService, | |||
There was a problem hiding this comment.
BaseControllerAdvice extends AbstractBaseController. Since the RestSecurityService lives in AbstractBaseController, yes. Otherwise, we define separate member variables in PdxBasedCrudController, CommonCrudController, and QueryAccessContoller.
There was a problem hiding this comment.
use Autowire inside AbstractBaseController to auto wire the RestSecurityService instead of using constructors.
Removed functions. Removed postProcess(query, regions, results) method Cleaned up testing
|
precheckin successful |
|
A few changes requested:
|
Do you think it would be better to only add the RestSecurityService to the controllers that actually need it instead of adding it to the base class? I think that would complicate the constructors. 2 & 3. Agreed, that'll make the test cleaner. |
Added a new PostProcessor to do some redaction.
Restore the ugly if/else block
|
Precheckin successful |
* This closes apache#276 (cherry picked from commit bd229d7)
Add post processing to the GET {region}/{key..key} endpoint
precheckin running