Skip to content

Commit e7cd132

Browse files
kahatlenkahatlen
committed
Fix tags when using JDBCHistoryCache.
Since the list of tags is already cached in the Repository instance, stop duplicating that information in the database. Instead, get that information from the Repository instance when retrieving history from the cache. Add a test case to verify that tags now work with JDBCHistoryCache. Also fix a bug in the hg log template (MercurialRepository.DIR_TEMPLATE), revealed by the added test case: A missing space in the template makes the last modified file and the first copied file appear as a single token (such as abc.txtdef.txt instead of abc.txt and def.txt) and causes inconsistencies in the history.
1 parent c221b36 commit e7cd132

File tree

15 files changed

+70
-98
lines changed

15 files changed

+70
-98
lines changed

‎src/org/opensolaris/opengrok/history/JDBCHistoryCache.java

Lines changed: 14 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class JDBCHistoryCache implements HistoryCache {
6060

6161
/** The names of all the tables created by this class. */
6262
private static final String[] TABLES = {
63-
"REPOSITORIES", "FILES", "AUTHORS", "TAGS", "CHANGESETS", "FILECHANGES",
63+
"REPOSITORIES", "FILES", "AUTHORS", "CHANGESETS", "FILECHANGES",
6464
"DIRECTORIES", "DIRCHANGES"
6565
};
6666

@@ -93,9 +93,6 @@ class JDBCHistoryCache implements HistoryCache {
9393
/** The id to be used for the next row inserted into AUTHORS. */
9494
private final AtomicInteger nextAuthorId = new AtomicInteger();
9595

96-
/** The id to be used for the next row inserted into TAGS. */
97-
private final AtomicInteger nextTagId = new AtomicInteger();
98-
9996
/** Info string to return from {@link #getInfo()}. */
10097
private String info;
10198

@@ -212,10 +209,6 @@ private void initDB(Statement s) throws SQLException {
212209
s.execute(getQuery("createTableAuthors"));
213210
}
214211

215-
if (!tableExists(dmd, SCHEMA, "TAGS")) {
216-
s.execute(getQuery("createTableTags"));
217-
}
218-
219212
if (!tableExists(dmd, SCHEMA, "CHANGESETS")) {
220213
s.execute(getQuery("createTableChangesets"));
221214
// Create a composite index on the repository in ascending order
@@ -242,7 +235,6 @@ private void initDB(Statement s) throws SQLException {
242235
initIdGenerator(s, "getMaxDirId", nextDirId);
243236
initIdGenerator(s, "getMaxChangesetId", nextChangesetId);
244237
initIdGenerator(s, "getMaxAuthorId", nextAuthorId);
245-
initIdGenerator(s, "getMaxTagId", nextTagId);
246238

247239
StringBuilder infoBuilder = new StringBuilder();
248240
infoBuilder.append(getClass().getSimpleName() + "\n");
@@ -557,17 +549,16 @@ private History getHistory(
557549
// Get the information about a changeset
558550
String revision = rs.getString(1);
559551
String author = rs.getString(2);
560-
String tags = rs.getString(3);
561-
Timestamp time = rs.getTimestamp(4);
562-
String message = rs.getString(5);
552+
Timestamp time = rs.getTimestamp(3);
553+
String message = rs.getString(4);
563554
HistoryEntry entry = new HistoryEntry(
564-
revision, time, author, tags, message, true);
555+
revision, time, author, null, message, true);
565556
entries.add(entry);
566557

567558
// Fill the list of files touched by the changeset, if
568559
// requested.
569560
if (withFiles) {
570-
int changeset = rs.getInt(6);
561+
int changeset = rs.getInt(5);
571562
filePS.setInt(1, changeset);
572563
try (ResultSet fileRS = filePS.executeQuery()) {
573564
while (fileRS.next()) {
@@ -583,6 +574,12 @@ private History getHistory(
583574

584575
History history = new History();
585576
history.setHistoryEntries(entries);
577+
578+
RuntimeEnvironment env = RuntimeEnvironment.getInstance();
579+
if (env.isTagsEnabled() && repository.hasFileBasedTags()) {
580+
repository.assignTagsInHistory(history);
581+
}
582+
586583
return history;
587584
}
588585

@@ -622,7 +619,6 @@ private void storeHistory(ConnectionResource conn, History history,
622619

623620
Integer reposId = null;
624621
Map<String, Integer> authors = null;
625-
Map<String, Integer> tags = null;
626622
Map<String, Integer> files = null;
627623
Map<String, Integer> directories = null;
628624
PreparedStatement addChangeset = null;
@@ -640,11 +636,6 @@ private void storeHistory(ConnectionResource conn, History history,
640636
authors = getAuthors(conn, history, reposId);
641637
conn.commit();
642638
}
643-
644-
if (tags == null) {
645-
tags = getTags(conn, history, reposId);
646-
conn.commit();
647-
}
648639

649640
if (directories == null || files == null) {
650641
Map<String, Integer> dirs = new HashMap<String, Integer>();
@@ -694,22 +685,17 @@ private void storeHistory(ConnectionResource conn, History history,
694685
try {
695686
addChangeset.setString(2, entry.getRevision());
696687
addChangeset.setInt(3, authors.get(entry.getAuthor()));
697-
if (entry.getTags() != null) {
698-
addChangeset.setInt(4, tags.get(entry.getTags()));
699-
} else {
700-
addChangeset.setNull(4, java.sql.Types.INTEGER);
701-
}
702-
addChangeset.setTimestamp(5,
688+
addChangeset.setTimestamp(4,
703689
new Timestamp(entry.getDate().getTime()));
704690
String msg = entry.getMessage();
705691
// Truncate the message if it can't fit in a VARCHAR
706692
// (bug #11663).
707693
if (msg.length() > MAX_MESSAGE_LENGTH) {
708694
msg = truncate(msg, MAX_MESSAGE_LENGTH);
709695
}
710-
addChangeset.setString(6, msg);
696+
addChangeset.setString(5, msg);
711697
int changesetId = nextChangesetId.getAndIncrement();
712-
addChangeset.setInt(7, changesetId);
698+
addChangeset.setInt(6, changesetId);
713699
addChangeset.executeUpdate();
714700

715701
// Add one row for each file in FILECHANGES, and one row
@@ -891,12 +877,6 @@ private int getRepositoryId(ConnectionResource conn, Repository repository)
891877
private static final InsertQuery ADD_AUTHOR =
892878
new InsertQuery(getQuery("addAuthor"));
893879

894-
private static final PreparedQuery GET_TAGS =
895-
new PreparedQuery(getQuery("getTags"));
896-
897-
private static final InsertQuery ADD_TAGS =
898-
new InsertQuery(getQuery("addTags"));
899-
900880
/**
901881
* Get a map from author names to their ids in the database. The authors
902882
* that are not in the database are added to it.
@@ -934,35 +914,6 @@ private Map<String, Integer> getAuthors(
934914

935915
return map;
936916
}
937-
938-
private Map<String, Integer> getTags(
939-
ConnectionResource conn, History history, int reposId)
940-
throws SQLException {
941-
HashMap<String, Integer> map = new HashMap<String, Integer>();
942-
PreparedStatement ps = conn.getStatement(GET_TAGS);
943-
ps.setInt(1, reposId);
944-
try (ResultSet rs = ps.executeQuery()) {
945-
while (rs.next()) {
946-
map.put(rs.getString(1), rs.getInt(2));
947-
}
948-
}
949-
950-
PreparedStatement insert = conn.getStatement(ADD_TAGS);
951-
insert.setInt(1, reposId);
952-
for (HistoryEntry entry : history.getHistoryEntries()) {
953-
String tags = entry.getTags();
954-
if (tags != null && !map.containsKey(tags)) {
955-
int id = nextTagId.getAndIncrement();
956-
insert.setString(2, tags);
957-
insert.setInt(3, id);
958-
insert.executeUpdate();
959-
map.put(tags, id);
960-
conn.commit();
961-
}
962-
}
963-
964-
return map;
965-
}
966917

967918
private static final PreparedQuery GET_DIRS =
968919
new PreparedQuery(getQuery("getDirectories"));

‎src/org/opensolaris/opengrok/history/JDBCHistoryCache_queries.properties

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,6 @@ createTableAuthors=\
5858
NAME VARCHAR(32672), \
5959
CONSTRAINT AUTHORS_REPO_NAME_UNIQUE UNIQUE (REPOSITORY, NAME))
6060

61-
createTableTags=\
62-
CREATE TABLE OPENGROK.TAGS (\
63-
ID INT CONSTRAINT TAGS_PK PRIMARY KEY, \
64-
REPOSITORY INT NOT NULL \
65-
CONSTRAINT TAGS_FK_REPO \
66-
REFERENCES OPENGROK.REPOSITORIES ON DELETE CASCADE, \
67-
NAME VARCHAR(32672), \
68-
CONSTRAINT TAGS_REPO_NAME_UNIQUE UNIQUE (REPOSITORY, NAME))
69-
7061
createTableChangesets=\
7162
CREATE TABLE OPENGROK.CHANGESETS (\
7263
ID INT CONSTRAINT CHANGESETS_PK PRIMARY KEY, \
@@ -77,9 +68,6 @@ createTableChangesets=\
7768
AUTHOR INT NOT NULL \
7869
CONSTRAINT CHANGESETS_FK_AUTH \
7970
REFERENCES OPENGROK.AUTHORS ON DELETE CASCADE, \
80-
TAG INT \
81-
CONSTRAINT CHANGESETS_FK_TAG \
82-
REFERENCES OPENGROK.TAGS ON DELETE CASCADE, \
8371
TIME TIMESTAMP NOT NULL, \
8472
MESSAGE VARCHAR(32672) NOT NULL, \
8573
CONSTRAINT CHANGESETS_REPO_REV_UNIQUE UNIQUE (REPOSITORY, REVISION))
@@ -113,28 +101,26 @@ hasCacheForDirectory=\
113101
WHERE D.REPOSITORY = R.ID AND R.PATH = ? AND D.PATH = ?
114102

115103
getFileHistory=\
116-
SELECT CS.REVISION, A.NAME, T.NAME, CS.TIME, CS.MESSAGE, CS.ID \
104+
SELECT CS.REVISION, A.NAME, CS.TIME, CS.MESSAGE, CS.ID \
117105
FROM \
118106
OPENGROK.CHANGESETS CS \
119107
JOIN OPENGROK.FILECHANGES FC ON CS.ID = FC.CHANGESET \
120108
JOIN OPENGROK.FILES F ON FC.FILE = F.ID \
121109
JOIN OPENGROK.DIRECTORIES D ON D.ID = F.DIRECTORY \
122110
JOIN OPENGROK.AUTHORS A ON A.ID = CS.AUTHOR \
123111
JOIN OPENGROK.REPOSITORIES R ON D.REPOSITORY = R.ID \
124-
LEFT JOIN OPENGROK.TAGS T ON T.ID = CS.TAG \
125112
WHERE \
126113
R.PATH = ? AND D.PATH = ? AND F.NAME = ? \
127114
ORDER BY CS.ID DESC
128115

129116
getDirHistory=\
130-
SELECT CS.REVISION, A.NAME, T.NAME, CS.TIME, CS.MESSAGE, CS.ID \
117+
SELECT CS.REVISION, A.NAME, CS.TIME, CS.MESSAGE, CS.ID \
131118
FROM \
132119
OPENGROK.CHANGESETS CS \
133120
JOIN OPENGROK.DIRCHANGES DC ON CS.ID = DC.CHANGESET \
134121
JOIN OPENGROK.DIRECTORIES D ON DC.DIRECTORY = D.ID \
135122
JOIN OPENGROK.AUTHORS A ON A.ID = CS.AUTHOR \
136123
JOIN OPENGROK.REPOSITORIES R ON D.REPOSITORY = R.ID \
137-
LEFT JOIN OPENGROK.TAGS T ON T.ID = CS.TAG \
138124
WHERE \
139125
R.PATH = ? AND D.PATH = ? \
140126
ORDER BY CS.ID DESC
@@ -150,8 +136,8 @@ addRepository=INSERT INTO OPENGROK.REPOSITORIES(PATH) VALUES ?
150136

151137
addChangeset=\
152138
INSERT INTO OPENGROK.CHANGESETS \
153-
(REPOSITORY, REVISION, AUTHOR, TAG, TIME, MESSAGE, ID) \
154-
VALUES (?,?,?,?,?,?,?)
139+
(REPOSITORY, REVISION, AUTHOR, TIME, MESSAGE, ID) \
140+
VALUES (?,?,?,?,?,?)
155141

156142
addDirchange=INSERT INTO OPENGROK.DIRCHANGES(CHANGESET, DIRECTORY) VALUES (?,?)
157143

@@ -161,10 +147,6 @@ getAuthors=SELECT NAME, ID FROM OPENGROK.AUTHORS WHERE REPOSITORY = ?
161147

162148
addAuthor=INSERT INTO OPENGROK.AUTHORS (REPOSITORY, NAME, ID) VALUES (?,?,?)
163149

164-
getTags=SELECT NAME, ID FROM OPENGROK.TAGS WHERE REPOSITORY = ?
165-
166-
addTags=INSERT INTO OPENGROK.TAGS (REPOSITORY, NAME, ID) VALUES (?,?,?)
167-
168150
getDirectories=SELECT PATH, ID FROM OPENGROK.DIRECTORIES WHERE REPOSITORY = ?
169151

170152
getFiles=\
@@ -190,8 +172,6 @@ getMaxChangesetId=SELECT MAX(ID) FROM OPENGROK.CHANGESETS
190172

191173
getMaxAuthorId=SELECT MAX(ID) FROM OPENGROK.AUTHORS
192174

193-
getMaxTagId=SELECT MAX(ID) FROM OPENGROK.TAGS
194-
195175
# Cascading deletes will take care of deleting all data associated with the
196176
# repository from the other tables.
197177
clearRepository=DELETE FROM OPENGROK.REPOSITORIES WHERE ID=?

‎src/org/opensolaris/opengrok/history/MercurialRepository.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public class MercurialRepository extends Repository {
7474

7575
/** Template for formatting hg log output for directories. */
7676
private static final String DIR_TEMPLATE = TEMPLATE
77-
+ "files: {files}{file_copies}\\n";
77+
+ "files: {files} {file_copies}\\n";
7878

7979
public MercurialRepository() {
8080
type = "Mercurial";

‎test/org/opensolaris/opengrok/history/JDBCHistoryCacheTest.java

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2009, 2012, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2009, 2013, Oracle and/or its affiliates. All rights reserved.
2222
*/
2323

2424
package org.opensolaris.opengrok.history;
@@ -37,6 +37,8 @@
3737
import junit.framework.Test;
3838
import junit.framework.TestCase;
3939
import junit.framework.TestSuite;
40+
import org.opensolaris.opengrok.configuration.Configuration;
41+
import org.opensolaris.opengrok.configuration.RuntimeEnvironment;
4042
import org.opensolaris.opengrok.util.Executor;
4143
import org.opensolaris.opengrok.util.TestRepository;
4244

@@ -100,6 +102,9 @@ public static Test suite() {
100102
throw sqle;
101103
}
102104
}
105+
106+
// Reset any changes the test made to the runtime environment.
107+
RuntimeEnvironment.getInstance().setConfiguration(new Configuration());
103108
}
104109

105110
/**
@@ -220,7 +225,7 @@ public void testStoreAndGet() throws Exception {
220225
History updatedHistory = cache.get(reposRoot, repos, true);
221226

222227
HistoryEntry newEntry = new HistoryEntry(
223-
"9:41e110bfdfb9",
228+
"10:1e392ef0b0ed",
224229
new Date(1245446973L / 60 * 60 * 1000), // whole minutes only
225230
"xyz", null, "Return failure when executed with no arguments",
226231
true);
@@ -269,7 +274,7 @@ public void testGetLatestCachedRevision() throws Exception {
269274
importHgChangeset(
270275
reposRoot, getClass().getResource("hg-export.txt").getPath());
271276
repos.createCache(cache, latestRevision);
272-
assertEquals("9:41e110bfdfb9", cache.getLatestCachedRevision(repos));
277+
assertEquals("10:1e392ef0b0ed", cache.getLatestCachedRevision(repos));
273278
}
274279

275280
/**
@@ -433,4 +438,40 @@ public void testNullAuthor() throws Exception {
433438
entries,
434439
cache.get(reposRoot, r, true).getHistoryEntries());
435440
}
441+
442+
/**
443+
* Test that tags work. Issue #101.
444+
*/
445+
public void testTags() throws Exception {
446+
// Enable tags
447+
RuntimeEnvironment.getInstance().setTagsEnabled(true);
448+
449+
File reposRoot = new File(repositories.getSourceRoot(), "mercurial");
450+
Repository repos = RepositoryFactory.getRepository(reposRoot);
451+
History historyToStore = repos.getHistory(reposRoot);
452+
cache.store(historyToStore, repos);
453+
cache.optimize();
454+
455+
List<HistoryEntry> dirHistory =
456+
cache.get(reposRoot, repos, false).getHistoryEntries();
457+
assertEquals("Size of history", 10, dirHistory.size());
458+
assertEquals("tip", dirHistory.get(0).getTags());
459+
assertNull(dirHistory.get(1).getTags());
460+
assertEquals("start_of_novel", dirHistory.get(2).getTags());
461+
assertNull(dirHistory.get(3).getTags());
462+
463+
List<HistoryEntry> novelHistory =
464+
cache.get(new File(reposRoot, "novel.txt"),
465+
repos, false).getHistoryEntries();
466+
assertEquals("Size of history", 2, novelHistory.size());
467+
assertEquals("tip", novelHistory.get(0).getTags());
468+
assertEquals("start_of_novel", novelHistory.get(1).getTags());
469+
470+
List<HistoryEntry> maincHistory =
471+
cache.get(new File(reposRoot, "main.c"),
472+
repos, false).getHistoryEntries();
473+
assertEquals("Size of history", 2, maincHistory.size());
474+
assertEquals("tip, start_of_novel", maincHistory.get(0).getTags());
475+
assertNull(maincHistory.get(1).getTags());
476+
}
436477
}

‎test/org/opensolaris/opengrok/history/MercurialRepositoryTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright 2010 Sun Microsystems, Inc. All rights reserved.
22-
* Use is subject to license terms.
21+
* Copyright (c) 2009, 2013, Oracle and/or its affiliates. All rights reserved.
2322
*/
2423

2524
package org.opensolaris.opengrok.history;
@@ -44,6 +43,7 @@ public class MercurialRepositoryTest {
4443
* that is latest changeset first.
4544
*/
4645
private static final String[] REVISIONS = {
46+
"9:8b340409b3a8",
4747
"8:6a8c423f5624", "7:db1394c05268", "6:e386b51ddbcc",
4848
"5:8706402863c6", "4:e494d67af12f", "3:2058725c1470",
4949
"2:585a1b3f2efb", "1:f24a5fd7a85d", "0:816b6279ae9c"

‎testdata/repositories/mercurial/.hgtags

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
db1394c05268686f89fbe4ca5524ac4ed61ec71c start_of_novel

‎testdata/repositories/mercurial/hg/cache/branchheads

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
6a8c423f56243a2ad988a8cc1980c222d4ec359d 8
2-
6a8c423f56243a2ad988a8cc1980c222d4ec359d default
1+
8b340409b3a81618750aa660cb113c65440688be 9
2+
8b340409b3a81618750aa660cb113c65440688be default

‎testdata/repositories/mercurial/hg/dirstate

24 Bytes
Binary file not shown.

‎testdata/repositories/mercurial/hg/last-message.txt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1 @@
1-
first words of the novel
2-
1+
Added tag start_of_novel for changeset db1394c05268

‎testdata/repositories/mercurial/hg/store/00changelog.i

221 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)