Skip to content
This repository has been archived by the owner on Jul 7, 2020. It is now read-only.

Jiayang/gdpr #289

Merged
merged 4 commits into from
Nov 6, 2018
Merged

Jiayang/gdpr #289

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
24cab17
added a refreshInterval field for AbstractMatchStringFilter
jiayangODC Oct 24, 2018
937e773
import only necessary resources
jiayangODC Oct 24, 2018
097bca9
updated implementation based on Ted's comments
jiayangODC Oct 25, 2018
41b2a71
update based on Ted's comment
jiayangODC Oct 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
updated implementation based on Ted's comments
  • Loading branch information
@jiayangODC
jiayangODC committed Oct 25, 2018
commit 097bca908883de4cd082607723fa7fe361d6a564
32 changes: 13 additions & 19 deletions ...-filters/src/main/java/com/addthis/hydra/data/filter/value/AbstractMatchStringFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
import java.util.regex.Pattern;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.ScheduledExecutorService;

import com.addthis.ahocorasick.AhoCorasick;
import com.addthis.ahocorasick.SearchResult;
Expand Down Expand Up @@ -107,7 +106,7 @@ abstract class AbstractMatchStringFilter extends AbstractValueFilterContextual i
/**
* Set an interval to fetch the required URL and update the bundle filter setting by calling the postDecodeHelper.
*/
final private int refreshInterval;
final private int refreshMinutes;

final private int urlMinBackoff;

Expand All @@ -132,7 +131,7 @@ abstract class AbstractMatchStringFilter extends AbstractValueFilterContextual i
boolean toLower,
int urlTimeout,
int urlRetries,
int refreshInterval,
int refreshMinutes,
int urlMinBackoff,
int urlMaxBackoff,
boolean not) {
Expand All @@ -148,7 +147,7 @@ abstract class AbstractMatchStringFilter extends AbstractValueFilterContextual i
this.toLower = toLower;
this.urlTimeout = urlTimeout;
this.urlRetries = urlRetries;
this.refreshInterval = refreshInterval;
this.refreshMinutes = refreshMinutes;
this.urlMinBackoff = urlMinBackoff;
this.urlMaxBackoff = urlMaxBackoff;
this.not = not;
Expand Down Expand Up @@ -270,20 +269,15 @@ public void postDecodeHelper() {
}

@Override public void postDecode() {
if (refreshInterval == 0) {
if (refreshMinutes == 0) {
postDecodeHelper();
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed a pretty serious issue here - in the case where refresh is enabled, we only call postDecodeHelper in the scheduled executor service. While it's true that we call it immediately, it's called asynchronously without blocking. We need to block here so that this value filter can't be used until the post decode step has occured at least once. Therefore, we should always call postDecodeHelper in this method, and change the scheduled executor to not refresh immediately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

ThreadPoolExecutor executor = (ThreadPoolExecutor) Executors.newFixedThreadPool(5);
ScheduledThreadPoolExecutor executor = new ScheduledThreadPoolExecutor(1);
// upon JVM termination, wait for the tasks for up to 100ms before exiting the executor service
ExecutorService executorService = MoreExecutors.getExitingExecutorService(executor, 100, TimeUnit.MILLISECONDS);

executorService.submit(() -> {
while (true) {
postDecodeHelper();
// sleep for refreshInterval minutes before decoding again
Thread.sleep(refreshInterval * 60 * 1000 );
}
});
ScheduledExecutorService executorService = MoreExecutors.getExitingScheduledExecutorService(executor, 100, TimeUnit.MILLISECONDS);
executorService.scheduleWithFixedDelay(() -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intellij inspections recommend using a method reference instead of a lambda here: this::postDecodeHelper

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

postDecodeHelper();
}, 0, refreshMinutes, TimeUnit.MINUTES);
}
}

Expand All @@ -302,7 +296,7 @@ public ValidationOnly(@AutoParam @JsonProperty("value") TypedField<Set<String>>
@JsonProperty("toLower") boolean toLower,
@Time(TimeUnit.MILLISECONDS) @JsonProperty("urlTimeout") int urlTimeout,
@JsonProperty("urlRetries") int urlRetries,
@Time(TimeUnit.MINUTES) @JsonProperty("refreshInterval") int refreshInterval,
@Time(TimeUnit.MINUTES) @JsonProperty("refreshMinutes") int refreshMinutes,
@Time(TimeUnit.MILLISECONDS) @JsonProperty("urlMinBackoff") int urlMinBackoff,
@Time(TimeUnit.MILLISECONDS) @JsonProperty("urlMaxBackoff") int urlMaxBackoff) {
super(value,
Expand All @@ -317,7 +311,7 @@ public ValidationOnly(@AutoParam @JsonProperty("value") TypedField<Set<String>>
toLower,
urlTimeout,
urlRetries,
refreshInterval,
refreshMinutes,
urlMinBackoff,
urlMaxBackoff,
false);
Expand Down
4 changes: 2 additions & 2 deletions hydra-filters/src/main/java/com/addthis/hydra/data/filter/value/ValueFilterExclude.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public class ValueFilterExclude extends AbstractMatchStringFilter {
@JsonProperty("toLower") boolean toLower,
@Time(TimeUnit.MILLISECONDS) @JsonProperty("urlTimeout") int urlTimeout,
@JsonProperty("urlRetries") int urlRetries,
@Time(TimeUnit.MINUTES) @JsonProperty("refreshInterval") int refreshInterval,
@Time(TimeUnit.MINUTES) @JsonProperty("refreshMinutes") int refreshMinutes,
@Time(TimeUnit.MILLISECONDS) @JsonProperty("urlMinBackoff") int urlMinBackoff,
@Time(TimeUnit.MILLISECONDS) @JsonProperty("urlMaxBackoff") int urlMaxBackoff) {
super(value,
Expand All @@ -71,7 +71,7 @@ public class ValueFilterExclude extends AbstractMatchStringFilter {
toLower,
urlTimeout,
urlRetries,
refreshInterval,
refreshMinutes,
urlMinBackoff,
urlMaxBackoff,
true);
Expand Down
4 changes: 2 additions & 2 deletions hydra-filters/src/main/java/com/addthis/hydra/data/filter/value/ValueFilterRequire.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public class ValueFilterRequire extends AbstractMatchStringFilter {
@JsonProperty("toLower") boolean toLower,
@Time(TimeUnit.MILLISECONDS) @JsonProperty("urlTimeout") int urlTimeout,
@JsonProperty("urlRetries") int urlRetries,
@Time(TimeUnit.MINUTES) @JsonProperty("refreshInterval") int refreshInterval,
@Time(TimeUnit.MINUTES) @JsonProperty("refreshMinutes") int refreshMinutes,
@Time(TimeUnit.MILLISECONDS) @JsonProperty("urlMinBackoff") int urlMinBackoff,
@Time(TimeUnit.MILLISECONDS) @JsonProperty("urlMaxBackoff") int urlMaxBackoff
) {
Expand All @@ -82,7 +82,7 @@ public class ValueFilterRequire extends AbstractMatchStringFilter {
toLower,
urlTimeout,
urlRetries,
refreshInterval,
refreshMinutes,
urlMinBackoff,
urlMaxBackoff,
false);
Expand Down