fix: Retrieving logentries pagewise always results in an exception (#…

…1220) * fix: Retrieving logentries pagewise always results in an exception * Address PR comments * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
googleapis · Dec 7, 2022 · 662a439 · 662a439
1 parent f4c346e
commit 662a439
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 19 deletions.
diff --git a/google-cloud-logging/src/main/java/com/google/cloud/logging/Logging.java b/google-cloud-logging/src/main/java/com/google/cloud/logging/Logging.java
@@ -1203,7 +1203,8 @@ ApiFuture<AsyncPage<MonitoredResourceDescriptor>> listMonitoredResourceDescripto
    * {@link EntryListOption#pageToken(String)} to specify the page token from which to start listing
    * entries. Use {@link EntryListOption#sortOrder(SortingField, SortingOrder)} to sort log entries
    * according to your preferred order (default is most-recent last). Use {@link
-   * EntryListOption#filter(String)} to filter listed log entries.
+   * EntryListOption#filter(String)} to filter listed log entries. By default a 24 hour filter is
+   * applied.
    *
    * <p>Example of listing log entries for a specific log.
    *
@@ -1231,7 +1232,8 @@ ApiFuture<AsyncPage<MonitoredResourceDescriptor>> listMonitoredResourceDescripto
    * specify the page size. Use {@link EntryListOption#pageToken(String)} to specify the page token
    * from which to start listing entries. Use {@link EntryListOption#sortOrder(SortingField,
    * SortingOrder)} to sort log entries according to your preferred order (default is most-recent
-   * last). Use {@link EntryListOption#filter(String)} to filter listed log entries.
+   * last). Use {@link EntryListOption#filter(String)} to filter listed log entries. By default a 24
+   * hour filter is applied.
    *
    * <p>Example of asynchronously listing log entries for a specific log.
    *

diff --git a/google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingImpl.java b/google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingImpl.java
@@ -1014,31 +1014,19 @@ static ListLogEntriesRequest listLogEntriesRequest(
     if (orderBy != null) {
       builder.setOrderBy(orderBy);
     }
-    String filter = EntryListOption.OptionType.FILTER.get(options);
-    // Make sure timestamp filter is either explicitly specified or we add a default
-    // time filter
-    // of 24 hours back to be inline with gcloud behavior for the same API
+    String filter = generateFilter(EntryListOption.OptionType.FILTER.get(options));
     if (filter != null) {
-      if (!Ascii.toLowerCase(filter).contains("timestamp")) {
-        filter =
-            String.format(
-                "%s AND %s", filter, defaultTimestampFilterCreator.createDefaultTimestampFilter());
-      }
       builder.setFilter(filter);
-    } else {
-      // If filter is not specified, default filter is looking back 24 hours in line
-      // with gcloud
-      // behavior
-      builder.setFilter(defaultTimestampFilterCreator.createDefaultTimestampFilter());
     }
-
     return builder.build();
   }
 
   private static ApiFuture<AsyncPage<LogEntry>> listLogEntriesAsync(
       final LoggingOptions serviceOptions, final Map<Option.OptionType, ?> options) {
+    // Make sure to set a filter option which later can be reused in subsequent calls
+    final Map<Option.OptionType, ?> updatedOptions = updateFilter(options);
     final ListLogEntriesRequest request =
-        listLogEntriesRequest(serviceOptions.getProjectId(), options);
+        listLogEntriesRequest(serviceOptions.getProjectId(), updatedOptions);
     ApiFuture<ListLogEntriesResponse> list = serviceOptions.getLoggingRpcV2().list(request);
     return transform(
         list,
@@ -1055,7 +1043,7 @@ public AsyncPage<LogEntry> apply(ListLogEntriesResponse listLogEntriesResponse)
                     ? null
                     : listLogEntriesResponse.getNextPageToken();
             return new AsyncPageImpl<>(
-                new LogEntryPageFetcher(serviceOptions, cursor, options), cursor, entries);
+                new LogEntryPageFetcher(serviceOptions, cursor, updatedOptions), cursor, entries);
           }
         });
   }
@@ -1137,6 +1125,37 @@ public void close() throws Exception {
     return optionMap;
   }
 
+  static Map<Option.OptionType, ?> updateFilter(final Map<Option.OptionType, ?> options) {
+    // We should see if filter provided in otiopns have a timestamp parameter
+    // and if not, it should be added with further update of options map.
+    String existingFilter = EntryListOption.OptionType.FILTER.get(options);
+    String newFilter = generateFilter(existingFilter);
+    if (newFilter.equals(existingFilter)) {
+      return options;
+    }
+    // Update
+    Map<Option.OptionType, Object> optionsCopy = Maps.newHashMap(options);
+    optionsCopy.put(EntryListOption.OptionType.FILTER, newFilter);
+    return optionsCopy;
+  }
+
+  static String generateFilter(String filter) {
+    String newFilter = filter;
+    // Make sure timestamp filter is either explicitly specified or we add a default
+    // time filter of 24 hours back to be inline with gcloud behavior for the same API
+    if (newFilter != null) {
+      if (!Ascii.toLowerCase(filter).contains("timestamp")) {
+        newFilter =
+            String.format(
+                "%s AND %s",
+                newFilter, defaultTimestampFilterCreator.createDefaultTimestampFilter());
+      }
+    } else {
+      newFilter = defaultTimestampFilterCreator.createDefaultTimestampFilter();
+    }
+    return newFilter;
+  }
+
   @VisibleForTesting
   int getNumPendingWrites() {
     return pendingWrites.size();

diff --git a/google-cloud-logging/src/test/java/com/google/cloud/logging/LoggingTest.java b/google-cloud-logging/src/test/java/com/google/cloud/logging/LoggingTest.java
@@ -18,6 +18,9 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
 
 import com.google.cloud.MonitoredResource;
 import com.google.cloud.logging.Logging.EntryListOption;
@@ -27,6 +30,7 @@
 import com.google.cloud.logging.Logging.WriteOption;
 import com.google.common.collect.ImmutableMap;
 import com.google.logging.v2.ListLogEntriesRequest;
+import java.util.Map;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -100,6 +104,18 @@ public void testEntryListOption() {
             "folders/test-folder");
   }
 
+  @Test
+  public void testFilterUpdate() {
+    Map<Option.OptionType, ?> options = LoggingImpl.optionMap(EntryListOption.filter(FILTER));
+    assertThat((String) EntryListOption.OptionType.FILTER.get(options)).isEqualTo(FILTER);
+    Map<Option.OptionType, ?> updated = LoggingImpl.updateFilter(options);
+    assertTrue(((String) EntryListOption.OptionType.FILTER.get(updated)).contains("timestamp"));
+    assertFalse(options == updated);
+    assertNotEquals(EntryListOption.OptionType.FILTER.get(updated), FILTER);
+    Map<Option.OptionType, ?> anotherUpdated = LoggingImpl.updateFilter(updated);
+    assertTrue(anotherUpdated == updated);
+  }
+
   @Test
   public void testWriteOption() {
     WriteOption writeOption = WriteOption.labels(LABELS);