opendev/gerrit
Code Issues Proposed changes

Fix Schema_151 upgrade by making createdOn nullable

Browse Source 
Otherwise, creating the column on a non-empty database fails due to
existing data being null:

 Exception in thread "main" com.google.gwtorm.server.OrmException: Cannot apply SQL
 ALTER TABLE account_groups ADD created_on TIMESTAMP NOT NULL
         at com.google.gwtorm.jdbc.JdbcExecutor.execute(JdbcExecutor.java:44)
         at com.google.gwtorm.schema.sql.DialectH2.addColumn(DialectH2.java:86)

Data for most groups should be filled in later by the actual schema
upgrade code, but the SQL to add the column needs to run before that.

Continue not returning null from AccountGroup#getCreatedOn, just
returning the default timestamp value if the underlying value is null.

Bug: Issue 6398
Change-Id: I182118ccc7a4ef231ba9f6420ddfe0afdfaac86f
This commit is contained in:
Dave Borowitz 2017-06-02 13:10:08 -04:00
parent faf43dbe14
commit 3d3799ce7b
4 changed files with 48 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountGroup.java
View File

@ -21,6 +21,18 @@ import java.sql.Timestamp;
/** Named group of one or more accounts, typically used for access controls. */
public final class AccountGroup {
/**
* Time when the audit subsystem was implemented, used as the default value for {@link #createdOn}
* when one couldn't be determined from the audit log.
*/
// Can't use Instant here because GWT. This is verified against a readable time in the tests,
// which don't need to compile under GWT.
private static final long AUDIT_CREATION_INSTANT_MS = 1244489460000L;
public static Timestamp auditCreationInstantTs() {
return new Timestamp(AUDIT_CREATION_INSTANT_MS);
}
/** Group name key */
public static class NameKey extends StringKey<com.google.gwtorm.client.Key<?>> {
private static final long serialVersionUID = 1L;
@ -146,7 +158,7 @@ public final class AccountGroup {
@Column(id = 10)
protected UUID ownerGroupUUID;
@Column(id = 11)
@Column(id = 11, notNull = false)
protected Timestamp createdOn;
protected AccountGroup() {}
@ -213,7 +225,7 @@ public final class AccountGroup {
}
public Timestamp getCreatedOn() {
return createdOn;
return createdOn != null ? createdOn : auditCreationInstantTs();
}
public void setCreatedOn(Timestamp createdOn) {

32
gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/AccountGroupTest.java Normal file
View File

@ -0,0 +1,32 @@
// Copyright (C) 2017 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.reviewdb.client;
import static com.google.common.truth.Truth.assertThat;
import java.sql.Timestamp;
import java.time.Instant;
import java.time.LocalDateTime;
import java.time.Month;
import java.time.ZoneOffset;
import org.junit.Test;
public class AccountGroupTest {
@Test
public void auditCreationInstant() {
Instant instant = LocalDateTime.of(2009, Month.JUNE, 8, 19, 31).toInstant(ZoneOffset.UTC);
assertThat(AccountGroup.auditCreationInstantTs()).isEqualTo(Timestamp.from(instant));
}
}

11
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_151.java
View File

@ -14,7 +14,6 @@
package com.google.gerrit.server.schema;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Streams;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupMemberAudit;
@ -25,20 +24,12 @@ import com.google.gwtorm.server.ResultSet;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.sql.Timestamp;
import java.time.Instant;
import java.time.LocalDateTime;
import java.time.Month;
import java.time.ZoneOffset;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
/** A schema which adds the 'created on' field to groups. */
public class Schema_151 extends SchemaVersion {
@VisibleForTesting
static final Instant AUDIT_CREATION_INSTANT =
LocalDateTime.of(2009, Month.JUNE, 8, 19, 31).toInstant(ZoneOffset.UTC);
@Inject
protected Schema_151(Provider<Schema_150> prior) {
super(prior);
@ -56,7 +47,7 @@ public class Schema_151 extends SchemaVersion {
.map(Key::getAddedOn)
.min(Comparator.naturalOrder());
Timestamp createdOn =
firstTimeMentioned.orElseGet(() -> Timestamp.from(AUDIT_CREATION_INSTANT));
firstTimeMentioned.orElseGet(() -> AccountGroup.auditCreationInstantTs());
accountGroup.setCreatedOn(createdOn);
}

2
gerrit-server/src/test/java/com/google/gerrit/server/schema/Schema_150_to_151_Test.java
View File

@ -132,7 +132,7 @@ public class Schema_150_to_151_Test {
schema151.migrateData(db, new TestUpdateUI());
AccountGroup group = db.accountGroups().get(groupId);
assertThat(group.getCreatedOn()).isEqualTo(Timestamp.from(Schema_151.AUDIT_CREATION_INSTANT));
assertThat(group.getCreatedOn()).isEqualTo(AccountGroup.auditCreationInstantTs());
}
private AccountGroup.Id createGroup(String name) throws Exception {