[GitHub] [nifi] simonbence commented on a change in pull request #5372: NIFI-9200: Free cache on heap after disabling AbstractCSVLookupService

[GitBox <gi...@apache.org>]

simonbence commented on a change in pull request #5372:
URL: https://github.com/apache/nifi/pull/5372#discussion_r704510002



##########
File path: nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/test/java/org/apache/nifi/lookup/TestCSVRecordLookupService.java
##########
@@ -111,5 +111,21 @@ public void testCsvRecordLookupServiceWithCustomSeparatorQuotedEscaped() throws
         assertEquals("my_value with an escaped |.", my_key.get().getAsString("value"));
     }
 
+    @Test
+    public void testCacheIsClearedWhenDisableService() throws InitializationException {
+        final TestRunner runner = TestRunners.newTestRunner(TestProcessor.class);

Review comment:
       Thanks for adding the tests, but please consider the following approach:
   Step 1: Using the service in order to have cache content. 
   Step 2: Check for the cache record by asking for it. The service should return the cached entry.
   Step 3: Disable and enable the service.
   Step 4: Check for the cache record by asking for it (again). The service should _not_ return any entry
   
   I think this approach is more subtle and describes the case more. That decision, that the cache is not emptied for example but thrown away by assigning null to the attribute is something internal to the implementation.




-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org