Merge remote-tracking branch 'AleksSPb/gh213_suspicious_activities_null_string'

Fix #213

Author: php-coder

src/main/java/ru/mystamps/web/entity/SuspiciousActivity.java

@@ -64,10 +64,10 @@ public class SuspiciousActivity {
 	@Column(length = IP_LENGTH, nullable = false)
 	private String ip;
 
-	@Column(name = "referer_page", length = REFERER_PAGE_LENGTH, nullable = false)
+	@Column(name = "referer_page", length = REFERER_PAGE_LENGTH)
 	private String refererPage;
 
-	@Column(name = "user_agent", length = USER_AGENT_LENGTH, nullable = false)
+	@Column(name = "user_agent", length = USER_AGENT_LENGTH)
 	private String userAgent;
 
 }

src/main/java/ru/mystamps/web/service/SiteServiceImpl.java

@@ -104,8 +104,8 @@ private void logEvent(
 		activity.setUser(user);
 
 		activity.setIp(StringUtils.defaultString(ip));
-		activity.setRefererPage(abbreviateRefererPage(StringUtils.defaultString(referer)));
-		activity.setUserAgent(abbreviateUserAgent(StringUtils.defaultString(agent)));
+		activity.setRefererPage(StringUtils.stripToNull(abbreviateRefererPage(referer)));
+		activity.setUserAgent(StringUtils.stripToNull(abbreviateUserAgent(agent)));
 
 		suspiciousActivities.add(activity);
 	}

src/main/resources/liquibase/version/0.4.xml

@@ -9,5 +9,6 @@
 	<include file="0.4/2015-06-22--image_url.xml" relativeToChangelogFile="true" />
 	<include file="0.4/2015-07-07--salt_and_hash.xml" relativeToChangelogFile="true" />
 	<include file="0.4/2015-10-14--http-method.xml" relativeToChangelogFile="true" />
+	<include file="0.4/2015-11-13--nullable_columns.xml" relativeToChangelogFile="true" />
 
 </databaseChangeLog>

src/main/resources/liquibase/version/0.4/2015-11-13--nullable_columns.xml

@@ -0,0 +1,41 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<databaseChangeLog
+		xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+		xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+		xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
+		http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.0.xsd">
+	
+	<changeSet id="allow-nullable-values-to-suspicious_activities-table" author="AleksSPb" context="scheme">
+		
+		<dropNotNullConstraint
+			columnDataType="VARCHAR(255)"
+			columnName="referer_page"
+			tableName="suspicious_activities"/>
+		
+		<dropNotNullConstraint
+			columnDataType="VARCHAR(255)"
+			columnName="user_agent"
+			tableName="suspicious_activities"/>
+		
+	</changeSet>
+	
+	<changeSet id="update-empty-values-to-null-in-suspicious_activities-table" author="AleksSPb" context="test-data, prod-data">
+		
+		<!-- Be caution: there is no rollback! -->
+		<sql>
+			UPDATE suspicious_activities
+			SET referer_page = NULL
+			WHERE TRIM(referer_page) = ''
+		</sql>
+		
+		<!-- Be caution: there is no rollback! -->
+		<sql>
+			UPDATE suspicious_activities
+			SET user_agent = NULL
+			WHERE TRIM(user_agent) = ''
+		</sql>
+		
+	</changeSet>
+	
+</databaseChangeLog>
+

src/test/groovy/ru/mystamps/web/service/SiteServiceImplTest.groovy

@@ -183,14 +183,18 @@ class SiteServiceImplTest extends Specification {
 			})
 	}
 
-	def "logAboutAbsentPage() should pass empty string to dao for unknown referer"() {
+	def "logAboutAbsentPage() should pass null to dao for unknown referer"(String refererPage) {
 		when:
-			service.logAboutAbsentPage(TEST_PAGE, TEST_METHOD, null, null, null, null)
+			service.logAboutAbsentPage(TEST_PAGE, TEST_METHOD, null, null, refererPage, null)
 		then:
 			1 * suspiciousActivityDao.add({ SuspiciousActivity activity ->
-				assert activity?.refererPage?.empty
+				assert activity?.refererPage == null
 				return true
 			})
+		where: refererPage | _
+			'  '           | _
+			''             | _
+			null           | _
 	}
 
 	def "logAboutAbsentPage() should pass user agent to dao"() {
@@ -216,14 +220,18 @@ class SiteServiceImplTest extends Specification {
 			})
 	}
 
-	def "logAboutAbsentPage() should pass empty string to dao for unknown user agent"() {
+	def "logAboutAbsentPage() should pass null to dao for unknown user agent"(String userAgent) {
 		when:
-			service.logAboutAbsentPage(TEST_PAGE, TEST_METHOD, null, null, null, null)
+			service.logAboutAbsentPage(TEST_PAGE, TEST_METHOD, null, null, null, userAgent)
 		then:
 			1 * suspiciousActivityDao.add({ SuspiciousActivity activity ->
-				assert activity?.userAgent?.empty
+				assert activity?.userAgent == null
 				return true
 			})
+		where: userAgent | _
+		'  '             | _
+		''               | _
+		null             | _
 	}
 
 	//
@@ -353,14 +361,18 @@ class SiteServiceImplTest extends Specification {
 			})
 	}
 
-	def "logAboutFailedAuthentication() should pass empty string to dao for unknown referer"() {
+	def "logAboutFailedAuthentication() should pass null to dao for unknown referer"(String refererPage) {
 		when:
-			service.logAboutFailedAuthentication(TEST_PAGE, TEST_METHOD, null, null, null, null, null)
+			service.logAboutFailedAuthentication(TEST_PAGE, TEST_METHOD, null, null, refererPage, null, null)
 		then:
 			1 * suspiciousActivityDao.add({ SuspiciousActivity activity ->
-				assert activity?.refererPage?.empty
+				assert activity?.refererPage == null
 				return true
 			})
+		where: refererPage | _
+			'  '           | _
+			''             | _
+			null           | _
 	}
 
 	def "logAboutFailedAuthentication() should pass abbreviated referer when it's too long"() {
@@ -387,14 +399,18 @@ class SiteServiceImplTest extends Specification {
 			})
 	}
 
-	def "logAboutFailedAuthentication() should pass empty string to dao for unknown user agent"() {
+	def "logAboutFailedAuthentication() should pass null to dao for unknown user agent"(String userAgent) {
 		when:
-			service.logAboutFailedAuthentication(TEST_PAGE, TEST_METHOD, null, null, null, null, null)
+			service.logAboutFailedAuthentication(TEST_PAGE, TEST_METHOD, null, null, null, userAgent, null)
 		then:
 			1 * suspiciousActivityDao.add({ SuspiciousActivity activity ->
-				assert activity?.userAgent?.empty
+				assert activity?.userAgent == null
 				return true
 			})
+		where: userAgent | _
+			'  '         | _
+			''           | _
+			null         | _
 	}
 
 	def "logAboutFailedAuthentication() should pass abbreviated user agent when it's too long"() {