From 16270daef411c907b5a1a24375023d0361e07390 Mon Sep 17 00:00:00 2001 From: Alex Panov Date: Mon, 29 Jun 2015 12:53:31 +0200 Subject: [PATCH 1/4] Fix issue #129, bump groovy-all version --- zuul-core/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zuul-core/build.gradle b/zuul-core/build.gradle index c1bc017e5f..bea3aaa0ce 100644 --- a/zuul-core/build.gradle +++ b/zuul-core/build.gradle @@ -2,7 +2,7 @@ apply plugin: 'groovy' dependencies { compile 'commons-io:commons-io:2.4' - compile 'org.codehaus.groovy:groovy-all:2.3.1' + compile 'org.codehaus.groovy:groovy-all:2.3.6' compile 'org.mockito:mockito-all:1.9.5' compile 'org.slf4j:slf4j-api:1.7.6' From ee4413047c24b40da100bb095284e5d6d713199d Mon Sep 17 00:00:00 2001 From: Alex Panov Date: Mon, 29 Jun 2015 13:52:19 +0200 Subject: [PATCH 2/4] Make FilterInfo immutable, remove superfluous JavaDoc, add tests --- .../zuul/scriptManager/FilterInfo.java | 66 +++++++++++-------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterInfo.java b/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterInfo.java index 9915afc566..6a1be03eb4 100644 --- a/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterInfo.java +++ b/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterInfo.java @@ -15,11 +15,16 @@ */ package com.netflix.zuul.scriptManager; -import net.jcip.annotations.ThreadSafe; - import java.util.Date; import java.util.concurrent.atomic.AtomicBoolean; +import org.junit.Test; + +import net.jcip.annotations.ThreadSafe; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; + /** * Representation of a ZuulFilter for representing and storing in a database */ @@ -34,20 +39,13 @@ public class FilterInfo implements Comparable{ private final String filter_order; private final String application_name; private int revision; - private Date creationDate; + private Date creationDate = new Date(); /* using AtomicBoolean so we can pass it into EndpointScriptMonitor */ private final AtomicBoolean isActive = new AtomicBoolean(); private final AtomicBoolean isCanary = new AtomicBoolean(); /** * Constructor - * @param filter_id - * @param filter_code - * @param filter_type - * @param filter_name - * @param disablePropertyName - * @param filter_order - * @param application_name */ public FilterInfo(String filter_id, String filter_code, String filter_type, String filter_name, String disablePropertyName, String filter_order, String application_name) { this.filter_id = filter_id; @@ -117,24 +115,10 @@ public String getApplication_name() { return application_name; } - /** - * - * @param filter_id - * @param revision - * @param creationDate - * @param isActive - * @param isCanary - * @param filter_code - * @param filter_type - * @param filter_name - * @param disablePropertyName - * @param filter_order - * @param application_name - */ public FilterInfo(String filter_id, int revision, Date creationDate, boolean isActive, boolean isCanary, String filter_code, String filter_type, String filter_name, String disablePropertyName, String filter_order, String application_name) { this.filter_id = filter_id; this.revision = revision; - this.creationDate = creationDate; + this.creationDate = new Date(creationDate.getTime()); this.isActive.set(isActive); this.isCanary.set(isCanary); this.filter_code = filter_code; @@ -159,7 +143,7 @@ public int getRevision() { * @return creation date */ public Date getCreationDate() { - return creationDate; + return new Date(creationDate.getTime()); } /** @@ -247,4 +231,34 @@ public int compareTo(FilterInfo filterInfo) { return filterInfo.getFilterName().compareTo(this.getFilterName()); } + public static class UnitTest { + + @Test + public void verifyFilterId() { + FilterInfo filterInfo = new FilterInfo("", "", "", "", "", "", ""); + long originalCreationTime = filterInfo.getCreationDate().getTime(); + filterInfo.getCreationDate().setTime(0); + assertThat(filterInfo.getCreationDate().getTime(), is(originalCreationTime)); + } + + @Test + public void creationDateIsCopiedInGetter() { + FilterInfo filterInfo = new FilterInfo("", "", "", "", "", "", ""); + long originalCreationTime = filterInfo.getCreationDate().getTime(); + filterInfo.getCreationDate().setTime(0); + assertThat(filterInfo.getCreationDate().getTime(), is(originalCreationTime)); + } + + @Test + public void creationDateIsCopiedInConstructor() { + Date date = new Date(); + long originalCreationTime = date.getTime(); + FilterInfo filterInfo = + new FilterInfo("", 1, date, false, false, "", "", "", "", "", ""); + date.setTime(0); + assertThat(filterInfo.getCreationDate().getTime(), is(originalCreationTime)); + } + + + } } From 7e423a2b6c48dc10fc9d3c644bf2c9b06aa8bb1a Mon Sep 17 00:00:00 2001 From: Alex Panov Date: Mon, 29 Jun 2015 13:52:36 +0200 Subject: [PATCH 3/4] Use imports in FilterVerifier --- .../com/netflix/zuul/scriptManager/FilterVerifier.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterVerifier.java b/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterVerifier.java index c776267bf2..68daa59efa 100644 --- a/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterVerifier.java +++ b/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterVerifier.java @@ -47,12 +47,12 @@ public static FilterVerifier getInstance() { * * @param sFilterCode * @return a FilterInfo object representing that code - * @throws org.codehaus.groovy.control.CompilationFailedException + * @throws CompilationFailedException * * @throws IllegalAccessException * @throws InstantiationException */ - public FilterInfo verifyFilter(String sFilterCode) throws org.codehaus.groovy.control.CompilationFailedException, IllegalAccessException, InstantiationException { + public FilterInfo verifyFilter(String sFilterCode) throws CompilationFailedException, IllegalAccessException, InstantiationException { Class groovyClass = compileGroovy(sFilterCode); Object instance = instanciateClass(groovyClass); checkZuulFilterInstance(instance); @@ -79,10 +79,10 @@ void checkZuulFilterInstance(Object zuulFilter) throws InstantiationException { * * @param sFilterCode * @return - * @throws org.codehaus.groovy.control.CompilationFailedException + * @throws CompilationFailedException * */ - public Class compileGroovy(String sFilterCode) throws org.codehaus.groovy.control.CompilationFailedException { + public Class compileGroovy(String sFilterCode) throws CompilationFailedException { GroovyClassLoader loader = new GroovyClassLoader(); return loader.parseClass(sFilterCode); } @@ -277,5 +277,3 @@ public void testVerify() { } } - - From 442ec659b2e3469975171afb4eb1b66aa0fcae55 Mon Sep 17 00:00:00 2001 From: Alex Panov Date: Mon, 29 Jun 2015 14:19:25 +0200 Subject: [PATCH 4/4] Extract FilterId concept --- .../main/java/com/netflix/zuul/FilterId.java | 84 +++++++++++++++++++ .../zuul/scriptManager/FilterInfo.java | 63 +++++++++----- .../zuul/scriptManager/FilterVerifier.java | 5 +- .../scriptManager/ZuulFilterDAOCassandra.java | 37 ++++++-- 4 files changed, 158 insertions(+), 31 deletions(-) create mode 100644 zuul-netflix/src/main/java/com/netflix/zuul/FilterId.java diff --git a/zuul-netflix/src/main/java/com/netflix/zuul/FilterId.java b/zuul-netflix/src/main/java/com/netflix/zuul/FilterId.java new file mode 100644 index 0000000000..f21a44b88d --- /dev/null +++ b/zuul-netflix/src/main/java/com/netflix/zuul/FilterId.java @@ -0,0 +1,84 @@ +package com.netflix.zuul; + +import java.util.UUID; + +import org.junit.After; +import org.junit.Test; + +import net.jcip.annotations.Immutable; +import net.jcip.annotations.ThreadSafe; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; + +@Immutable +@ThreadSafe +public final class FilterId { + + private String value; + + private FilterId(String value) { + this.value = value; + } + + @Override + public String toString() { + return value; + } + + public static class Builder { + private String applicationName = ZuulApplicationInfo.getApplicationName(); + private String filterType; + private String filterName; + + public Builder applicationName(String applicationName) { + this.applicationName = applicationName; + return this; + } + + public Builder filterType(String filterType) { + this.filterType = filterType; + return this; + } + + public Builder filterName(String filterName) { + this.filterName = filterName; + return this; + } + + public FilterId build() { + return new FilterId(applicationName + ":" + filterName + ":" + filterType); + } + } + + + public static class UnitTest { + + String zuulAppName = ZuulApplicationInfo.getApplicationName(); + + @After + public void setZuulAppNameBack() { + ZuulApplicationInfo.setApplicationName(zuulAppName); + } + + @Test + public void filterId() { + FilterId filterId = new Builder().applicationName("app") + .filterType("com.acme.None") + .filterName("none") + .build(); + assertThat(filterId.toString(), is("app:none:com.acme.None")); + } + + @Test + public void defaultApplicationName() { + String applicationName = UUID.randomUUID().toString(); + ZuulApplicationInfo.setApplicationName(applicationName); + + FilterId filterId = new Builder().filterType("com.acme.None") + .filterName("none") + .build(); + assertThat(filterId.toString(), is(applicationName + ":none:com.acme.None")); + } + } +} diff --git a/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterInfo.java b/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterInfo.java index 6a1be03eb4..889eae65b9 100644 --- a/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterInfo.java +++ b/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterInfo.java @@ -18,6 +18,10 @@ import java.util.Date; import java.util.concurrent.atomic.AtomicBoolean; +import com.netflix.zuul.FilterId; +import com.netflix.zuul.ZuulApplicationInfo; +import com.netflix.zuul.ZuulFilter; + import org.junit.Test; import net.jcip.annotations.ThreadSafe; @@ -28,6 +32,7 @@ /** * Representation of a ZuulFilter for representing and storing in a database */ + @ThreadSafe public class FilterInfo implements Comparable{ @@ -40,12 +45,13 @@ public class FilterInfo implements Comparable{ private final String application_name; private int revision; private Date creationDate = new Date(); + /* using AtomicBoolean so we can pass it into EndpointScriptMonitor */ private final AtomicBoolean isActive = new AtomicBoolean(); private final AtomicBoolean isCanary = new AtomicBoolean(); /** - * Constructor + * Constructors */ public FilterInfo(String filter_id, String filter_code, String filter_type, String filter_name, String disablePropertyName, String filter_order, String application_name) { this.filter_id = filter_id; @@ -59,6 +65,32 @@ public FilterInfo(String filter_id, String filter_code, String filter_type, Stri isCanary.set(false); } + public FilterInfo(String filterCode, String filterName, ZuulFilter filter) { + this.filter_code = filterCode; + this.filter_type = filter.filterType(); + this.filter_name = filterName; + this.filter_disablePropertyName = filter.disablePropertyName(); + this.filter_order = "" + filter.filterOrder(); + this.application_name = ZuulApplicationInfo.getApplicationName(); + isActive.set(false); + isCanary.set(false); + this.filter_id = buildFilterId(); + } + + public FilterInfo(String filter_id, int revision, Date creationDate, boolean isActive, boolean isCanary, String filter_code, String filter_type, String filter_name, String disablePropertyName, String filter_order, String application_name) { + this.filter_id = filter_id; + this.revision = revision; + this.creationDate = new Date(creationDate.getTime()); + this.isActive.set(isActive); + this.isCanary.set(isCanary); + this.filter_code = filter_code; + this.filter_name = filter_name; + this.filter_type = filter_type; + this.filter_order = filter_order; + this.filter_disablePropertyName = disablePropertyName; + this.application_name = application_name; + } + /** * * @return the filter name; the class name of the filter @@ -92,7 +124,6 @@ public String getFilterType() { return filter_type; } - @Override public String toString() { return "FilterInfo{" + @@ -115,21 +146,6 @@ public String getApplication_name() { return application_name; } - public FilterInfo(String filter_id, int revision, Date creationDate, boolean isActive, boolean isCanary, String filter_code, String filter_type, String filter_name, String disablePropertyName, String filter_order, String application_name) { - this.filter_id = filter_id; - this.revision = revision; - this.creationDate = new Date(creationDate.getTime()); - this.isActive.set(isActive); - this.isCanary.set(isCanary); - this.filter_code = filter_code; - this.filter_name = filter_name; - this.filter_type = filter_type; - this.filter_order = filter_order; - this.filter_disablePropertyName = disablePropertyName; - this.application_name = application_name; - - } - /** * * @return the revision of this filter @@ -163,7 +179,6 @@ public boolean isCanary() { return isCanary.get(); } - /** * * @return unique key for the filter @@ -188,7 +203,15 @@ public String getFilterOrder() { * @return key is application_name:filter_name:filter_type */ public static String buildFilterID(String application_name, String filter_type, String filter_name) { - return application_name + ":" + filter_name + ":" + filter_type; + return new FilterId.Builder().applicationName(application_name) + .filterType(filter_type) + .filterName(filter_name) + .build() + .toString(); + } + + public String buildFilterId() { + return buildFilterID(application_name, filter_type, filter_name); } @Override @@ -258,7 +281,5 @@ public void creationDateIsCopiedInConstructor() { date.setTime(0); assertThat(filterInfo.getCreationDate().getTime(), is(originalCreationTime)); } - - } } diff --git a/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterVerifier.java b/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterVerifier.java index 68daa59efa..4524c747a7 100644 --- a/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterVerifier.java +++ b/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/FilterVerifier.java @@ -58,10 +58,7 @@ public FilterInfo verifyFilter(String sFilterCode) throws CompilationFailedExcep checkZuulFilterInstance(instance); ZuulFilter filter = (ZuulFilter) instance; - - String filter_id = FilterInfo.buildFilterID(ZuulApplicationInfo.getApplicationName(), filter.filterType(), groovyClass.getSimpleName()); - - return new FilterInfo(filter_id, sFilterCode, filter.filterType(), groovyClass.getSimpleName(), filter.disablePropertyName(), "" + filter.filterOrder(), ZuulApplicationInfo.getApplicationName()); + return new FilterInfo(sFilterCode, groovyClass.getSimpleName(), filter); } Object instanciateClass(Class groovyClass) throws InstantiationException, IllegalAccessException { diff --git a/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/ZuulFilterDAOCassandra.java b/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/ZuulFilterDAOCassandra.java index e2130f4940..caef2de973 100644 --- a/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/ZuulFilterDAOCassandra.java +++ b/zuul-netflix/src/main/java/com/netflix/zuul/scriptManager/ZuulFilterDAOCassandra.java @@ -15,18 +15,30 @@ */ package com.netflix.zuul.scriptManager; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Calendar; +import java.util.Collections; +import java.util.Date; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Observable; + import com.netflix.astyanax.AstyanaxContext; import com.netflix.astyanax.Keyspace; import com.netflix.astyanax.model.Column; import com.netflix.astyanax.model.ColumnList; import com.netflix.astyanax.model.Row; import com.netflix.astyanax.model.Rows; +import com.netflix.zuul.FilterId; import com.netflix.zuul.ZuulApplicationInfo; import com.netflix.zuul.dependency.cassandra.hystrix.HystrixCassandraGetRowsByKeys; import com.netflix.zuul.dependency.cassandra.hystrix.HystrixCassandraGetRowsByQuery; import com.netflix.zuul.dependency.cassandra.hystrix.HystrixCassandraPut; import com.netflix.zuul.event.ZuulEvent; -import net.jcip.annotations.ThreadSafe; + import org.junit.Before; import org.junit.Test; import org.mockito.InOrder; @@ -35,11 +47,22 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.*; +import net.jcip.annotations.ThreadSafe; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.anyString; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.anyList; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; /** * from zuul_filters @@ -318,8 +341,10 @@ public FilterInfo addFilter(String filtercode, String filter_type, String filter } public static String buildFilterID(String filter_type, String filter_name) { - return FilterInfo.buildFilterID(ZuulApplicationInfo.getApplicationName(), filter_type, filter_name); - + return new FilterId.Builder().filterType(filter_type) + .filterName(filter_name) + .build() + .toString(); } @Override