diff --git a/.bazelversion b/.bazelversion index ccbccc3dc6..4a36342fca 100644 --- a/.bazelversion +++ b/.bazelversion @@ -1 +1 @@ -2.2.0 +3.0.0 diff --git a/.settings/edu.umd.cs.findbugs.core.prefs b/.settings/edu.umd.cs.findbugs.core.prefs deleted file mode 100644 index 4dfcf2d11d..0000000000 --- a/.settings/edu.umd.cs.findbugs.core.prefs +++ /dev/null @@ -1,143 +0,0 @@ -#FindBugs User Preferences -#Fri Mar 20 17:07:10 JST 2015 -cloud_id=edu.umd.cs.findbugs.cloud.doNothingCloud -detectorAppendingToAnObjectOutputStream=AppendingToAnObjectOutputStream|true -detectorAtomicityProblem=AtomicityProblem|true -detectorBadAppletConstructor=BadAppletConstructor|false -detectorBadResultSetAccess=BadResultSetAccess|true -detectorBadSyntaxForRegularExpression=BadSyntaxForRegularExpression|true -detectorBadUseOfReturnValue=BadUseOfReturnValue|true -detectorBadlyOverriddenAdapter=BadlyOverriddenAdapter|true -detectorBooleanReturnNull=BooleanReturnNull|true -detectorCallToUnsupportedMethod=CallToUnsupportedMethod|false -detectorCheckExpectedWarnings=CheckExpectedWarnings|false -detectorCheckImmutableAnnotation=CheckImmutableAnnotation|true -detectorCheckRelaxingNullnessAnnotation=CheckRelaxingNullnessAnnotation|true -detectorCheckTypeQualifiers=CheckTypeQualifiers|true -detectorCloneIdiom=CloneIdiom|true -detectorComparatorIdiom=ComparatorIdiom|true -detectorConfusedInheritance=ConfusedInheritance|true -detectorConfusionBetweenInheritedAndOuterMethod=ConfusionBetweenInheritedAndOuterMethod|true -detectorCovariantArrayAssignment=CovariantArrayAssignment|false -detectorCrossSiteScripting=CrossSiteScripting|true -detectorDefaultEncodingDetector=DefaultEncodingDetector|true -detectorDoInsideDoPrivileged=DoInsideDoPrivileged|true -detectorDontCatchIllegalMonitorStateException=DontCatchIllegalMonitorStateException|true -detectorDontIgnoreResultOfPutIfAbsent=DontIgnoreResultOfPutIfAbsent|true -detectorDontUseEnum=DontUseEnum|true -detectorDroppedException=DroppedException|true -detectorDumbMethodInvocations=DumbMethodInvocations|true -detectorDumbMethods=DumbMethods|true -detectorDuplicateBranches=DuplicateBranches|true -detectorEmptyZipFileEntry=EmptyZipFileEntry|false -detectorEqualsOperandShouldHaveClassCompatibleWithThis=EqualsOperandShouldHaveClassCompatibleWithThis|true -detectorExplicitSerialization=ExplicitSerialization|true -detectorFinalizerNullsFields=FinalizerNullsFields|true -detectorFindBadCast2=FindBadCast2|true -detectorFindBadForLoop=FindBadForLoop|true -detectorFindCircularDependencies=FindCircularDependencies|false -detectorFindComparatorProblems=FindComparatorProblems|true -detectorFindDeadLocalStores=FindDeadLocalStores|true -detectorFindDoubleCheck=FindDoubleCheck|true -detectorFindEmptySynchronizedBlock=FindEmptySynchronizedBlock|true -detectorFindFieldSelfAssignment=FindFieldSelfAssignment|true -detectorFindFinalizeInvocations=FindFinalizeInvocations|true -detectorFindFloatEquality=FindFloatEquality|true -detectorFindHEmismatch=FindHEmismatch|true -detectorFindInconsistentSync2=FindInconsistentSync2|true -detectorFindJSR166LockMonitorenter=FindJSR166LockMonitorenter|true -detectorFindLocalSelfAssignment2=FindLocalSelfAssignment2|true -detectorFindMaskedFields=FindMaskedFields|true -detectorFindMismatchedWaitOrNotify=FindMismatchedWaitOrNotify|true -detectorFindNakedNotify=FindNakedNotify|true -detectorFindNonShortCircuit=FindNonShortCircuit|true -detectorFindNullDeref=FindNullDeref|true -detectorFindNullDerefsInvolvingNonShortCircuitEvaluation=FindNullDerefsInvolvingNonShortCircuitEvaluation|true -detectorFindOpenStream=FindOpenStream|true -detectorFindPuzzlers=FindPuzzlers|true -detectorFindRefComparison=FindRefComparison|true -detectorFindReturnRef=FindReturnRef|true -detectorFindRoughConstants=FindRoughConstants|true -detectorFindRunInvocations=FindRunInvocations|true -detectorFindSelfComparison=FindSelfComparison|true -detectorFindSelfComparison2=FindSelfComparison2|true -detectorFindSleepWithLockHeld=FindSleepWithLockHeld|true -detectorFindSpinLoop=FindSpinLoop|true -detectorFindSqlInjection=FindSqlInjection|true -detectorFindTwoLockWait=FindTwoLockWait|true -detectorFindUncalledPrivateMethods=FindUncalledPrivateMethods|true -detectorFindUnconditionalWait=FindUnconditionalWait|true -detectorFindUninitializedGet=FindUninitializedGet|true -detectorFindUnrelatedTypesInGenericContainer=FindUnrelatedTypesInGenericContainer|true -detectorFindUnreleasedLock=FindUnreleasedLock|true -detectorFindUnsatisfiedObligation=FindUnsatisfiedObligation|true -detectorFindUnsyncGet=FindUnsyncGet|true -detectorFindUseOfNonSerializableValue=FindUseOfNonSerializableValue|true -detectorFindUselessControlFlow=FindUselessControlFlow|true -detectorFindUselessObjects=FindUselessObjects|true -detectorFormatStringChecker=FormatStringChecker|true -detectorHugeSharedStringConstants=HugeSharedStringConstants|true -detectorIDivResultCastToDouble=IDivResultCastToDouble|true -detectorIncompatMask=IncompatMask|true -detectorInconsistentAnnotations=InconsistentAnnotations|true -detectorInefficientIndexOf=InefficientIndexOf|true -detectorInefficientInitializationInsideLoop=InefficientInitializationInsideLoop|false -detectorInefficientMemberAccess=InefficientMemberAccess|false -detectorInefficientToArray=InefficientToArray|true -detectorInfiniteLoop=InfiniteLoop|true -detectorInfiniteRecursiveLoop=InfiniteRecursiveLoop|true -detectorInheritanceUnsafeGetResource=InheritanceUnsafeGetResource|true -detectorInitializationChain=InitializationChain|true -detectorInitializeNonnullFieldsInConstructor=InitializeNonnullFieldsInConstructor|true -detectorInstantiateStaticClass=InstantiateStaticClass|true -detectorIntCast2LongAsInstant=IntCast2LongAsInstant|true -detectorInvalidJUnitTest=InvalidJUnitTest|true -detectorIteratorIdioms=IteratorIdioms|true -detectorLazyInit=LazyInit|true -detectorLoadOfKnownNullValue=LoadOfKnownNullValue|true -detectorLostLoggerDueToWeakReference=LostLoggerDueToWeakReference|true -detectorMethodReturnCheck=MethodReturnCheck|true -detectorMultithreadedInstanceAccess=MultithreadedInstanceAccess|true -detectorMutableEnum=MutableEnum|true -detectorMutableLock=MutableLock|true -detectorMutableStaticFields=MutableStaticFields|true -detectorNaming=Naming|true -detectorNoteUnconditionalParamDerefs=NoteUnconditionalParamDerefs|true -detectorNumberConstructor=NumberConstructor|true -detectorOptionalReturnNull=OptionalReturnNull|true -detectorOverridingEqualsNotSymmetrical=OverridingEqualsNotSymmetrical|true -detectorPreferZeroLengthArrays=PreferZeroLengthArrays|true -detectorPublicSemaphores=PublicSemaphores|false -detectorQuestionableBooleanAssignment=QuestionableBooleanAssignment|true -detectorReadOfInstanceFieldInMethodInvokedByConstructorInSuperclass=ReadOfInstanceFieldInMethodInvokedByConstructorInSuperclass|true -detectorReadReturnShouldBeChecked=ReadReturnShouldBeChecked|true -detectorRedundantConditions=RedundantConditions|true -detectorRedundantInterfaces=RedundantInterfaces|true -detectorRepeatedConditionals=RepeatedConditionals|true -detectorRuntimeExceptionCapture=RuntimeExceptionCapture|true -detectorSerializableIdiom=SerializableIdiom|true -detectorStartInConstructor=StartInConstructor|true -detectorStaticCalendarDetector=StaticCalendarDetector|true -detectorStringConcatenation=StringConcatenation|true -detectorSuperfluousInstanceOf=SuperfluousInstanceOf|true -detectorSuspiciousThreadInterrupted=SuspiciousThreadInterrupted|true -detectorSwitchFallthrough=SwitchFallthrough|true -detectorSynchronizationOnSharedBuiltinConstant=SynchronizationOnSharedBuiltinConstant|true -detectorSynchronizeAndNullCheckField=SynchronizeAndNullCheckField|true -detectorSynchronizeOnClassLiteralNotGetClass=SynchronizeOnClassLiteralNotGetClass|true -detectorSynchronizingOnContentsOfFieldToProtectField=SynchronizingOnContentsOfFieldToProtectField|true -detectorURLProblems=URLProblems|true -detectorUncallableMethodOfAnonymousClass=UncallableMethodOfAnonymousClass|true -detectorUnnecessaryMath=UnnecessaryMath|true -detectorUnreadFields=UnreadFields|true -detectorUselessSubclassMethod=UselessSubclassMethod|false -detectorVarArgsProblems=VarArgsProblems|true -detectorVolatileUsage=VolatileUsage|true -detectorWaitInLoop=WaitInLoop|true -detectorWrongMapIterator=WrongMapIterator|true -detectorXMLFactoryBypass=XMLFactoryBypass|true -detector_threshold=2 -effort=default -filter_settings=Medium|BAD_PRACTICE,CORRECTNESS,EXPERIMENTAL,MALICIOUS_CODE,MT_CORRECTNESS,PERFORMANCE,SECURITY,STYLE|false|12 -filter_settings_neg=NOISE,I18N| -run_at_full_build=false diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 7dcd97e756..3e36d691f0 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -706,6 +706,13 @@ If not absolute, the path is resolved relative to `$site_path`. + Default is unset, no disk cache. +[[cache.enableDiskStatMetrics]]cache.enableDiskStatMetrics:: ++ +Whether to enable the computation of disk statistics of persistent caches. +This computation is expensive and requires a long time on larger installations. ++ +By default, false. + [[cache.h2CacheSize]]cache.h2CacheSize:: + The size of the in-memory cache for each opened H2 cache database, in bytes. diff --git a/Documentation/dev-e2e-tests.txt b/Documentation/dev-e2e-tests.txt index 850631f21f..fa3eb263f6 100644 --- a/Documentation/dev-e2e-tests.txt +++ b/Documentation/dev-e2e-tests.txt @@ -189,7 +189,9 @@ sbt "gatling:lastReport" The `src/test/resources/logback.xml` file link:http://logback.qos.ch/manual/configuration.html[configures,role=external,window=_blank] -Gatling's logging level. +Gatling's logging level. To quickly +enable link:https://gatling.io/docs/current/general/debugging#logback[detailed logging] of `http` +requests and responses, the `root level` can be set to `trace` in that file. === How to run using Docker diff --git a/Documentation/intro-user.txt b/Documentation/intro-user.txt index 9dd58b8818..bf6bc77f41 100644 --- a/Documentation/intro-user.txt +++ b/Documentation/intro-user.txt @@ -114,12 +114,15 @@ explains the link:error-messages.html[error messages]. [[change-ref]] When a commit is pushed for review, Gerrit stores it in a staging area -which is a branch in the special `refs/changes/` namespace. A change -ref has the format `refs/changes/XX/YYYY/ZZ` where `YYYY` is the -numeric change number, `ZZ` is the patch set number and `XX` is the -last two digits of the numeric change number, e.g. -`refs/changes/20/884120/1`. Understanding the format of this ref is not -required for working with Gerrit. +which is a branch in the special `refs/changes/` namespace. Understanding +the format of this ref is not required for working with Gerrit, but it +is explained below. + +A change ref has the format `refs/changes/X/Y/Z` where `X` is the last +two digits of the change number, `Y` is the entire change number, and `Z` +is the patch set. For example, if the change number is +link:https://gerrit-review.googlesource.com/c/gerrit/+/263270[263270], +the ref would be `refs/changes/70/263270/2` for the second patch set. [[fetch-change]] Using the change ref git clients can fetch the corresponding commit, diff --git a/Documentation/licenses.txt b/Documentation/licenses.txt index 84b1bc89f1..63c569aab2 100644 --- a/Documentation/licenses.txt +++ b/Documentation/licenses.txt @@ -922,18 +922,6 @@ EXHIBIT A -Mozilla Public License. ---- -[[PublicDomain]] -PublicDomain - -* guice:aopalliance - -[[PublicDomain_license]] ----- -This software has been placed in the public domain by its author(s). - ----- - - [[antlr]] antlr diff --git a/Documentation/pg-plugin-endpoints.txt b/Documentation/pg-plugin-endpoints.txt index fe5dc06eeb..a8b333076a 100644 --- a/Documentation/pg-plugin-endpoints.txt +++ b/Documentation/pg-plugin-endpoints.txt @@ -156,6 +156,22 @@ link:rest-api-changes.html#change-info[ChangeInfo] The submit action, including the title and label, an instance of link:rest-api-changes.html#action-info[ActionInfo] +=== commit-container +The `commit-container` extension point adds content at the end of the commit +message to the change view. + +In addition to default parameters, the following are available: + +* `change` ++ +current change displayed, an instance of +link:rest-api-changes.html#change-info[ChangeInfo] + +* `revision` ++ +current revision displayed, an instance of +link:rest-api-changes.html#revision-info[RevisionInfo] + == Dynamic Plugin endpoints The following endpoints are available to plugins. diff --git a/Jenkinsfile b/Jenkinsfile index 5f93803f27..cafd654f38 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -90,16 +90,18 @@ def collectBuildModes() { def changedFiles = queryChangedFiles(Globals.gerritUrl) def polygerritFiles = changedFiles.findAll { it.startsWith("polygerrit-ui") || it.startsWith("lib/js") } + def bazelFiles = changedFiles.findAll { it == "WORKSPACE" || it.endsWith("BUILD") || + it.endsWith(".bzl") } if(polygerritFiles.size() > 0) { - if(changedFiles.size() == polygerritFiles.size()) { + if(changedFiles.size() == polygerritFiles.size() && bazelFiles.isEmpty()) { println "Only PolyGerrit UI changes detected, skipping other test modes..." Builds.modes = ["polygerrit"] } else { println "PolyGerrit UI changes detected, adding 'polygerrit' validation..." Builds.modes += "polygerrit" } - } else if(changedFiles.contains("WORKSPACE")) { + } else if(!bazelFiles.isEmpty()) { println "WORKSPACE file changes detected, adding 'polygerrit' validation..." Builds.modes += "polygerrit" } @@ -110,10 +112,10 @@ def prepareBuildsForMode(buildName, mode="notedb", retryTimes = 1) { stage("${buildName}/${mode}") { def slaveBuild = null for (int i = 1; i <= retryTimes; i++) { + postCheck(new GerritCheck( + (buildName == "Gerrit-codestyle") ? "codestyle" : mode, + new Build(currentBuild.getAbsoluteUrl(), null))) try { - postCheck(new GerritCheck( - (buildName == "Gerrit-codestyle") ? "codestyle" : mode, - new Build(currentBuild.getAbsoluteUrl(), null))) slaveBuild = build job: "${buildName}", parameters: [ string(name: 'REFSPEC', value: "refs/changes/${env.BRANCH_NAME}"), string(name: 'BRANCH', value: env.GERRIT_PATCHSET_REVISION), diff --git a/WORKSPACE b/WORKSPACE index 2c2631fbaa..97fadaf559 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -52,7 +52,10 @@ http_archive( name = "io_bazel_rules_closure", sha256 = "b9c2bc6ba377aa497eb7c31681d34404febf9d4e3c9c7d98ce0d78238a0af20f", strip_prefix = "rules_closure-0.31", - urls = ["https://github.com/davido/rules_closure/archive/V0.31.tar.gz"], + urls = [ + "https://github.com/davido/rules_closure/archive/V0.31.tar.gz", + "https://gerrit-ci.gerritforge.com/lib/V0.31.tar.gz", + ], ) http_archive( @@ -153,10 +156,20 @@ maven_jar( GUICE_VERS = "4.2.3" -maven_jar( - name = "guice-library", - artifact = "com.google.inject:guice:" + GUICE_VERS, - sha1 = "2ea992d6d7bdcac7a43111a95d182a4c42eb5ff7", +GUICE_LIBRARY_SHA256 = "5168f5e7383f978c1b4154ac777b78edd8ac214bb9f9afdb92921c8d156483d3" + +http_file( + name = "guice-library-no-aop", + canonical_id = "guice-library-no-aop-" + GUICE_VERS + ".jar-" + GUICE_LIBRARY_SHA256, + downloaded_file_path = "guice-library-no-aop.jar", + sha256 = GUICE_LIBRARY_SHA256, + urls = [ + "https://repo1.maven.org/maven2/com/google/inject/guice/" + + GUICE_VERS + + "/guice-" + + GUICE_VERS + + "-no_aop.jar", + ], ) maven_jar( @@ -171,12 +184,6 @@ maven_jar( sha1 = "8d6e7e35eac4fb5e7df19c55b3bc23fa51b10a11", ) -maven_jar( - name = "aopalliance", - artifact = "aopalliance:aopalliance:1.0", - sha1 = "0235ba8b489512805ac13a8f9ea77a1ca5ebe3e8", -) - maven_jar( name = "javax_inject", artifact = "javax.inject:javax.inject:1", @@ -257,14 +264,17 @@ maven_jar( sha1 = "6000774d7f8412ced005a704188ced78beeed2bb", ) +CAFFEINE_GUAVA_SHA256 = "3a66ee3ec70971dee0bae6e56bda7b8742bc4bedd7489161bfbbaaf7137d89e1" + # TODO(davido): Rename guava.jar to caffeine-guava.jar on fetch to prevent potential # naming collision between caffeine guava adapater and guava library itself. # Remove this renaming procedure, once this upstream issue is fixed: # https://github.com/ben-manes/caffeine/issues/364. http_file( name = "caffeine-guava-renamed", + canonical_id = "caffeine-guava-" + CAFFEINE_VERS + ".jar-" + CAFFEINE_GUAVA_SHA256, downloaded_file_path = "caffeine-guava-" + CAFFEINE_VERS + ".jar", - sha256 = "3a66ee3ec70971dee0bae6e56bda7b8742bc4bedd7489161bfbbaaf7137d89e1", + sha256 = CAFFEINE_GUAVA_SHA256, urls = [ "https://repo1.maven.org/maven2/com/github/ben-manes/caffeine/guava/" + CAFFEINE_VERS + diff --git a/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.json b/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.json index 86f9bf135b..23891246d1 100644 --- a/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.json +++ b/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.json @@ -1,26 +1,10 @@ [ { - "url": "ssh://admin@localhost:29418/loadtest-repo", + "url": "ssh://admin@HOSTNAME:SSH_PORT/_PROJECT", "cmd": "clone" }, { - "url": "ssh://admin@localhost:29418/loadtest-repo", - "cmd": "pull" - }, - { - "url": "ssh://admin@localhost:29418/loadtest-repo", - "cmd": "push" - }, - { - "url": "http://localhost:8080/loadtest-repo", + "url": "http://HOSTNAME:HTTP_PORT/_PROJECT", "cmd": "clone" - }, - { - "url": "http://localhost:8080/loadtest-repo", - "cmd": "pull" - }, - { - "url": "http://localhost:8080/loadtest-repo", - "cmd": "push" } ] diff --git a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala index 86a336de77..462358647a 100644 --- a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala +++ b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala @@ -28,7 +28,7 @@ class CloneUsingBothProtocols extends GitSimulation { replaceKeyWith("_project", default, in) } - private val test: ScenarioBuilder = scenario(name) + private val test: ScenarioBuilder = scenario(unique) .feed(data) .exec(gitRequest) @@ -40,11 +40,11 @@ class CloneUsingBothProtocols extends GitSimulation { atOnceUsers(1) ), test.inject( - nothingFor(1 second), + nothingFor(2 seconds), constantUsersPerSec(1) during (2 seconds) ), deleteProject.test.inject( - nothingFor(3 second), + nothingFor(6 seconds), atOnceUsers(1) ), ).protocols(gitProtocol, httpProtocol) diff --git a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateProject.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateProject.scala index 931ff020df..b6027ddf04 100644 --- a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateProject.scala +++ b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateProject.scala @@ -26,7 +26,7 @@ class CreateProject extends ProjectSimulation { this.default = default } - val test: ScenarioBuilder = scenario(name) + val test: ScenarioBuilder = scenario(unique) .feed(data) .exec(httpRequest) diff --git a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/DeleteProject.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/DeleteProject.scala index bf13f83c53..f17fbba497 100644 --- a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/DeleteProject.scala +++ b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/DeleteProject.scala @@ -26,7 +26,7 @@ class DeleteProject extends ProjectSimulation { this.default = default } - val test: ScenarioBuilder = scenario(name) + val test: ScenarioBuilder = scenario(unique) .feed(data) .exec(httpRequest) diff --git a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala index d47af01c0c..906633c30d 100644 --- a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala +++ b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala @@ -27,8 +27,9 @@ class GerritSimulation extends Simulation { private val path: String = pack.replaceAllLiterally(".", "/") protected val name: String = this.getClass.getSimpleName protected val resource: String = s"data/$path/$name.json" + protected val unique: String = name + "-" + this.hashCode() - protected val httpRequest: HttpRequestBuilder = http(name).post("${url}") + protected val httpRequest: HttpRequestBuilder = http(unique).post("${url}") protected val httpProtocol: HttpProtocolBuilder = http.basicAuth( conf.httpConfiguration.userName, conf.httpConfiguration.password) diff --git a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala index 32df1b5286..ff49b9a23e 100644 --- a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala +++ b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala @@ -21,21 +21,37 @@ import io.gatling.core.structure.ScenarioBuilder import scala.concurrent.duration._ class ReplayRecordsFromFeeder extends GitSimulation { - private val data: FileBasedFeederBuilder[Any]#F = jsonFile(resource).circular + private val data: FileBasedFeederBuilder[Any]#F#F = jsonFile(resource).convert(url).circular + private val default: String = name - private val test: ScenarioBuilder = scenario(name) - .repeat(10000) { + override def replaceOverride(in: String): String = { + replaceKeyWith("_project", default, in) + } + + private val test: ScenarioBuilder = scenario(unique) + .repeat(10) { feed(data) .exec(gitRequest) } + private val createProject = new CreateProject(default) + private val deleteProject = new DeleteProject(default) + setUp( + createProject.test.inject( + atOnceUsers(1) + ), test.inject( nothingFor(4 seconds), atOnceUsers(10), rampUsers(10) during (5 seconds), constantUsersPerSec(20) during (15 seconds), constantUsersPerSec(20) during (15 seconds) randomized - )).protocols(gitProtocol) - .maxDuration(60 seconds) + ), + deleteProject.test.inject( + nothingFor(59 seconds), + atOnceUsers(1) + ), + ).protocols(gitProtocol, httpProtocol) + .maxDuration(61 seconds) } diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index 2afdad99cc..8df9518650 100644 --- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -20,9 +20,13 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static com.google.common.truth.Truth8.assertThat; import static com.google.common.truth.TruthJUnit.assume; +import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow; +import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block; import static com.google.gerrit.entities.Patch.COMMIT_MSG; import static com.google.gerrit.entities.Patch.MERGE_LIST; import static com.google.gerrit.extensions.api.changes.SubmittedTogetherOption.NON_VISIBLE_CHANGES; +import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; +import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; import static com.google.gerrit.server.project.ProjectCache.illegalState; import static com.google.gerrit.server.project.testing.TestLabels.label; import static com.google.gerrit.server.project.testing.TestLabels.value; @@ -42,6 +46,7 @@ import com.google.common.primitives.Chars; import com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context; import com.google.gerrit.acceptance.PushOneCommit.Result; import com.google.gerrit.acceptance.testsuite.account.TestSshKeys; +import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.data.AccessSection; @@ -299,6 +304,7 @@ public abstract class AbstractDaemonTest { @Inject private ProjectIndexCollection projectIndexes; @Inject private RequestScopeOperations requestScopeOperations; @Inject private SitePaths sitePaths; + @Inject private ProjectOperations projectOperations; private ProjectResetter resetter; private List toClose; @@ -414,6 +420,9 @@ public abstract class AbstractDaemonTest { baseConfig.setString("sshd", null, "listenAddress", "off"); } + baseConfig.unset("gerrit", null, "canonicalWebUrl"); + baseConfig.unset("httpd", null, "listenUrl"); + baseConfig.setInt("index", null, "batchThreads", -1); baseConfig.setInt("receive", null, "changeUpdateThreads", 4); @@ -987,6 +996,16 @@ public abstract class AbstractDaemonTest { } } + protected void blockAnonymousRead() throws Exception { + String allRefs = RefNames.REFS + "*"; + projectOperations + .project(project) + .forUpdate() + .add(block(Permission.READ).ref(allRefs).group(ANONYMOUS_USERS)) + .add(allow(Permission.READ).ref(allRefs).group(REGISTERED_USERS)) + .update(); + } + protected PushOneCommit.Result pushTo(String ref) throws Exception { PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); return push.to(ref); diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java index b25dcc3c92..0ef6ad59d7 100644 --- a/java/com/google/gerrit/acceptance/GerritServer.java +++ b/java/com/google/gerrit/acceptance/GerritServer.java @@ -468,9 +468,13 @@ public class GerritServer implements AutoCloseable { private static void mergeTestConfig(Config cfg) { String forceEphemeralPort = String.format("%s:0", getLocalHost().getHostName()); String url = "http://" + forceEphemeralPort + "/"; - cfg.setString("gerrit", null, "canonicalWebUrl", url); - cfg.setString("httpd", null, "listenUrl", url); + if (cfg.getString("gerrit", null, "canonicalWebUrl") == null) { + cfg.setString("gerrit", null, "canonicalWebUrl", url); + } + if (cfg.getString("httpd", null, "listenUrl") == null) { + cfg.setString("httpd", null, "listenUrl", url); + } if (cfg.getString("sshd", null, "listenAddress") == null) { cfg.setString("sshd", null, "listenAddress", forceEphemeralPort); } diff --git a/java/com/google/gerrit/elasticsearch/ElasticVersion.java b/java/com/google/gerrit/elasticsearch/ElasticVersion.java index 2d023cf251..82816fb25f 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticVersion.java +++ b/java/com/google/gerrit/elasticsearch/ElasticVersion.java @@ -18,9 +18,6 @@ import com.google.common.base.Joiner; import java.util.regex.Pattern; public enum ElasticVersion { - V6_2("6.2.*"), - V6_3("6.3.*"), - V6_4("6.4.*"), V6_5("6.5.*"), V6_6("6.6.*"), V6_7("6.7.*"), diff --git a/java/com/google/gerrit/entities/Account.java b/java/com/google/gerrit/entities/Account.java index fd3df977f3..cd3b27abaf 100644 --- a/java/com/google/gerrit/entities/Account.java +++ b/java/com/google/gerrit/entities/Account.java @@ -269,4 +269,9 @@ public abstract class Account { public abstract Account build(); } + + @Override + public final String toString() { + return getName(); + } } diff --git a/java/com/google/gerrit/entities/Project.java b/java/com/google/gerrit/entities/Project.java index ecef87d69c..867b14db79 100644 --- a/java/com/google/gerrit/entities/Project.java +++ b/java/com/google/gerrit/entities/Project.java @@ -23,6 +23,7 @@ import java.io.Serializable; import java.util.Arrays; import java.util.HashMap; import java.util.Map; +import java.util.Optional; /** Projects match a source code repository managed by Gerrit */ public final class Project { @@ -247,4 +248,9 @@ public final class Project { public void setConfigRefState(String state) { configRefState = state; } + + @Override + public String toString() { + return Optional.of(getName()).orElse(""); + } } diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java index 284d8f60e7..8df53436d7 100644 --- a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java +++ b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java @@ -24,7 +24,6 @@ import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeMessageInfo; import com.google.gerrit.extensions.common.CommentInfo; import com.google.gerrit.extensions.common.CommitMessageInput; -import com.google.gerrit.extensions.common.EditInfo; import com.google.gerrit.extensions.common.MergePatchSetInput; import com.google.gerrit.extensions.common.PureRevertInfo; import com.google.gerrit.extensions.common.RevertSubmissionInfo; @@ -187,12 +186,6 @@ public interface ChangeApi { EnumSet listOptions, EnumSet submitOptions) throws RestApiException; - /** Publishes a draft change. */ - @Deprecated - default void publish() { - throw new UnsupportedOperationException("draft workflow is discontinued"); - } - /** Rebase the current revision of a change using default options. */ default void rebase() throws RestApiException { rebase(new RebaseInput()); @@ -272,17 +265,6 @@ public interface ChangeApi { return get(EnumSet.noneOf(ListChangesOption.class)); } - /** - * Retrieve change edit when exists. - * - * @deprecated Replaced by {@link ChangeApi#edit()} in combination with {@link - * ChangeEditApi#get()}. - */ - @Deprecated - default EditInfo getEdit() throws RestApiException { - return edit().get().orElse(null); - } - /** * Provides access to an API regarding the change edit of this change. * diff --git a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java index 7ae570f9fa..ff9fb3c559 100644 --- a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java +++ b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java @@ -38,11 +38,6 @@ import java.util.Map; import java.util.Set; public interface RevisionApi { - @Deprecated - default void delete() { - throw new UnsupportedOperationException("draft workflow is discontinued"); - } - String description() throws RestApiException; void description(String description) throws RestApiException; @@ -62,11 +57,6 @@ public interface RevisionApi { BinaryResult submitPreview(String format) throws RestApiException; - @Deprecated - default void publish() { - throw new UnsupportedOperationException("draft workflow is discontinued"); - } - ChangeApi cherryPick(CherryPickInput in) throws RestApiException; ChangeInfo cherryPickAsInfo(CherryPickInput in) throws RestApiException; diff --git a/java/com/google/gerrit/metrics/proc/OperatingSystemMXBeanFactory.java b/java/com/google/gerrit/metrics/proc/OperatingSystemMXBeanFactory.java index 9befe16a2b..ef0ced61c6 100644 --- a/java/com/google/gerrit/metrics/proc/OperatingSystemMXBeanFactory.java +++ b/java/com/google/gerrit/metrics/proc/OperatingSystemMXBeanFactory.java @@ -20,10 +20,11 @@ import java.lang.management.ManagementFactory; import java.lang.management.OperatingSystemMXBean; import java.util.Arrays; -@SuppressWarnings("restriction") class OperatingSystemMXBeanFactory { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private OperatingSystemMXBeanFactory() {} + static OperatingSystemMXBeanInterface create() { OperatingSystemMXBean sys = ManagementFactory.getOperatingSystemMXBean(); if (sys instanceof UnixOperatingSystemMXBean) { diff --git a/java/com/google/gerrit/metrics/proc/OperatingSystemMXBeanUnixNative.java b/java/com/google/gerrit/metrics/proc/OperatingSystemMXBeanUnixNative.java index a7f5bba9ef..fbde058d19 100644 --- a/java/com/google/gerrit/metrics/proc/OperatingSystemMXBeanUnixNative.java +++ b/java/com/google/gerrit/metrics/proc/OperatingSystemMXBeanUnixNative.java @@ -16,7 +16,6 @@ package com.google.gerrit.metrics.proc; import com.sun.management.UnixOperatingSystemMXBean; -@SuppressWarnings("restriction") class OperatingSystemMXBeanUnixNative implements OperatingSystemMXBeanInterface { private final UnixOperatingSystemMXBean sys; diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java index 2d3cf2f923..a831b8e1aa 100644 --- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java +++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java @@ -76,6 +76,7 @@ import com.google.gerrit.server.project.ProjectCacheImpl; import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.SubmitRuleEvaluator; import com.google.gerrit.server.query.change.ChangeData; +import com.google.gerrit.server.query.change.ChangeIsVisibleToPredicate; import com.google.gerrit.server.restapi.group.GroupModule; import com.google.gerrit.server.rules.DefaultSubmitRule; import com.google.gerrit.server.rules.IgnoreSelfApprovalRule; @@ -167,6 +168,7 @@ public class BatchProgramModule extends FactoryModule { install(PureRevertCache.module()); factory(CapabilityCollection.Factory.class); factory(ChangeData.AssistedFactory.class); + factory(ChangeIsVisibleToPredicate.Factory.class); factory(ProjectState.Factory.class); // Submit rules diff --git a/java/com/google/gerrit/server/cache/CacheMetrics.java b/java/com/google/gerrit/server/cache/CacheMetrics.java index 5d309525db..12194e75cd 100644 --- a/java/com/google/gerrit/server/cache/CacheMetrics.java +++ b/java/com/google/gerrit/server/cache/CacheMetrics.java @@ -25,10 +25,12 @@ import com.google.gerrit.metrics.CallbackMetric1; import com.google.gerrit.metrics.Description; import com.google.gerrit.metrics.Field; import com.google.gerrit.metrics.MetricMaker; +import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.logging.Metadata; import com.google.inject.Inject; import com.google.inject.Singleton; import java.util.Set; +import org.eclipse.jgit.lib.Config; @Singleton public class CacheMetrics { @@ -36,7 +38,8 @@ public class CacheMetrics { Field.ofString("cache_name", Metadata.Builder::cacheName).build(); @Inject - public CacheMetrics(MetricMaker metrics, DynamicMap> cacheMap) { + public CacheMetrics( + MetricMaker metrics, DynamicMap> cacheMap, @GerritServerConfig Config config) { CallbackMetric1 memEnt = metrics.newCallbackMetric( "caches/memory_cached", @@ -81,7 +84,8 @@ public class CacheMetrics { memEnt.set(name, c.size()); memHit.set(name, cstats.hitRate() * 100); memEvict.set(name, cstats.evictionCount()); - if (c instanceof PersistentCache) { + if (c instanceof PersistentCache + && config.getBoolean("cache", "enableDiskStatMetrics", false)) { PersistentCache.DiskStats d = ((PersistentCache) c).diskStats(); perDiskEnt.set(name, d.size()); perDiskHit.set(name, hitRatio(d)); diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java index 1f8075ab1f..e447f2b59e 100644 --- a/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -173,6 +173,7 @@ import com.google.gerrit.server.project.ProjectNameLockManager; import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.SubmitRuleEvaluator; import com.google.gerrit.server.query.change.ChangeData; +import com.google.gerrit.server.query.change.ChangeIsVisibleToPredicate; import com.google.gerrit.server.query.change.ChangeQueryBuilder; import com.google.gerrit.server.query.change.ConflictsCacheImpl; import com.google.gerrit.server.quota.QuotaEnforcer; @@ -263,6 +264,7 @@ public class GerritGlobalModule extends FactoryModule { factory(CapabilityCollection.Factory.class); factory(ChangeData.AssistedFactory.class); factory(ChangeJson.AssistedFactory.class); + factory(ChangeIsVisibleToPredicate.Factory.class); factory(LabelsJson.Factory.class); factory(MergeUtil.Factory.class); factory(PatchScriptFactory.Factory.class); diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java index 72a3f07694..7535f514f7 100644 --- a/java/com/google/gerrit/server/git/validators/CommitValidators.java +++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java @@ -141,23 +141,25 @@ public class CommitValidators { PermissionBackend.ForRef perm = forProject.ref(branch.branch()); ProjectState projectState = projectCache.get(branch.project()).orElseThrow(illegalState(branch.project())); - return new CommitValidators( - ImmutableList.of( - new UploadMergesPermissionValidator(perm), - new ProjectStateValidationListener(projectState), - new AmendedGerritMergeCommitValidationListener(perm, gerritIdent), - new AuthorUploaderValidator(user, perm, urlFormatter.get()), - new FileCountValidator(repoManager, config), - new CommitterUploaderValidator(user, perm, urlFormatter.get()), - new SignedOffByValidator(user, perm, projectState), + ImmutableList.Builder validators = ImmutableList.builder(); + validators + .add(new UploadMergesPermissionValidator(perm)) + .add(new ProjectStateValidationListener(projectState)) + .add(new AmendedGerritMergeCommitValidationListener(perm, gerritIdent)) + .add(new AuthorUploaderValidator(user, perm, urlFormatter.get())) + .add(new FileCountValidator(repoManager, config)) + .add(new CommitterUploaderValidator(user, perm, urlFormatter.get())) + .add(new SignedOffByValidator(user, perm, projectState)) + .add( new ChangeIdValidator( - projectState, user, urlFormatter.get(), config, sshInfo, change), - new ConfigValidator(projectConfigFactory, branch, user, rw, allUsers, allProjects), - new BannedCommitsValidator(rejectCommits), - new PluginCommitValidationListener(pluginValidators, skipValidation), - new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker), - new AccountCommitValidator(repoManager, allUsers, accountValidator), - new GroupCommitValidator(allUsers))); + projectState, user, urlFormatter.get(), config, sshInfo, change)) + .add(new ConfigValidator(projectConfigFactory, branch, user, rw, allUsers, allProjects)) + .add(new BannedCommitsValidator(rejectCommits)) + .add(new PluginCommitValidationListener(pluginValidators, skipValidation)) + .add(new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker)) + .add(new AccountCommitValidator(repoManager, allUsers, accountValidator)) + .add(new GroupCommitValidator(allUsers)); + return new CommitValidators(validators.build()); } public CommitValidators forGerritCommits( @@ -170,21 +172,23 @@ public class CommitValidators { PermissionBackend.ForRef perm = forProject.ref(branch.branch()); ProjectState projectState = projectCache.get(branch.project()).orElseThrow(illegalState(branch.project())); - return new CommitValidators( - ImmutableList.of( - new UploadMergesPermissionValidator(perm), - new ProjectStateValidationListener(projectState), - new AmendedGerritMergeCommitValidationListener(perm, gerritIdent), - new AuthorUploaderValidator(user, perm, urlFormatter.get()), - new FileCountValidator(repoManager, config), - new SignedOffByValidator(user, perm, projectState), + ImmutableList.Builder validators = ImmutableList.builder(); + validators + .add(new UploadMergesPermissionValidator(perm)) + .add(new ProjectStateValidationListener(projectState)) + .add(new AmendedGerritMergeCommitValidationListener(perm, gerritIdent)) + .add(new AuthorUploaderValidator(user, perm, urlFormatter.get())) + .add(new FileCountValidator(repoManager, config)) + .add(new SignedOffByValidator(user, perm, projectState)) + .add( new ChangeIdValidator( - projectState, user, urlFormatter.get(), config, sshInfo, change), - new ConfigValidator(projectConfigFactory, branch, user, rw, allUsers, allProjects), - new PluginCommitValidationListener(pluginValidators), - new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker), - new AccountCommitValidator(repoManager, allUsers, accountValidator), - new GroupCommitValidator(allUsers))); + projectState, user, urlFormatter.get(), config, sshInfo, change)) + .add(new ConfigValidator(projectConfigFactory, branch, user, rw, allUsers, allProjects)) + .add(new PluginCommitValidationListener(pluginValidators)) + .add(new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker)) + .add(new AccountCommitValidator(repoManager, allUsers, accountValidator)) + .add(new GroupCommitValidator(allUsers)); + return new CommitValidators(validators.build()); } public CommitValidators forMergedCommits( @@ -205,12 +209,13 @@ public class CommitValidators { PermissionBackend.ForRef perm = forProject.ref(branch.branch()); ProjectState projectState = projectCache.get(branch.project()).orElseThrow(illegalState(branch.project())); - return new CommitValidators( - ImmutableList.of( - new UploadMergesPermissionValidator(perm), - new ProjectStateValidationListener(projectState), - new AuthorUploaderValidator(user, perm, urlFormatter.get()), - new CommitterUploaderValidator(user, perm, urlFormatter.get()))); + ImmutableList.Builder validators = ImmutableList.builder(); + validators + .add(new UploadMergesPermissionValidator(perm)) + .add(new ProjectStateValidationListener(projectState)) + .add(new AuthorUploaderValidator(user, perm, urlFormatter.get())) + .add(new CommitterUploaderValidator(user, perm, urlFormatter.get())); + return new CommitValidators(validators.build()); } } diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java index 5aa45db1d8..22d332ace0 100644 --- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java +++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java @@ -366,7 +366,7 @@ public abstract class ChangeEmail extends NotificationEmail { } } - /** Users who have non-zero approval codes on the change. */ + /** Users who were added as reviewers to this change. */ protected void ccExistingReviewers() { if (!NotifyHandling.ALL.equals(notify.handling()) && !NotifyHandling.OWNER_REVIEWERS.equals(notify.handling())) { diff --git a/java/com/google/gerrit/server/project/CreateRefControl.java b/java/com/google/gerrit/server/project/CreateRefControl.java index 3c4af0b0b8..69ac93e839 100644 --- a/java/com/google/gerrit/server/project/CreateRefControl.java +++ b/java/com/google/gerrit/server/project/CreateRefControl.java @@ -29,6 +29,7 @@ import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; import java.io.IOException; +import java.util.Optional; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; @@ -76,7 +77,7 @@ public class CreateRefControl { PermissionBackend.ForRef perm = permissionBackend.user(user.get()).ref(branch); if (object instanceof RevCommit) { perm.check(RefPermission.CREATE); - checkCreateCommit(repo, (RevCommit) object, ps.getNameKey(), perm); + checkCreateCommit(user, repo, (RevCommit) object, ps.getNameKey(), perm); } else if (object instanceof RevTag) { RevTag tag = (RevTag) object; try (RevWalk rw = new RevWalk(repo)) { @@ -96,7 +97,7 @@ public class CreateRefControl { RevObject target = tag.getObject(); if (target instanceof RevCommit) { - checkCreateCommit(repo, (RevCommit) target, ps.getNameKey(), perm); + checkCreateCommit(user, repo, (RevCommit) target, ps.getNameKey(), perm); } else { checkCreateRef(user, repo, branch, target); } @@ -117,7 +118,11 @@ public class CreateRefControl { * new commit to the repository. */ private void checkCreateCommit( - Repository repo, RevCommit commit, Project.NameKey project, PermissionBackend.ForRef forRef) + Provider user, + Repository repo, + RevCommit commit, + Project.NameKey project, + PermissionBackend.ForRef forRef) throws AuthException, PermissionBackendException, IOException { try { // If the user has update (push) permission, they can create the ref regardless @@ -131,7 +136,8 @@ public class CreateRefControl { project, repo, commit, - repo.getRefDatabase().getRefsByPrefix(Constants.R_HEADS, Constants.R_TAGS))) { + repo.getRefDatabase().getRefsByPrefix(Constants.R_HEADS, Constants.R_TAGS), + Optional.of(user.get()))) { // If the user has no push permissions, check whether the object is // merged into a branch or tag readable by this user. If so, they are // not effectively "pushing" more objects, so they can create the ref diff --git a/java/com/google/gerrit/server/project/Reachable.java b/java/com/google/gerrit/server/project/Reachable.java index 57e9a7e729..331b7da741 100644 --- a/java/com/google/gerrit/server/project/Reachable.java +++ b/java/com/google/gerrit/server/project/Reachable.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.project; import com.google.common.flogger.FluentLogger; import com.google.gerrit.entities.Project; +import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.change.IncludedInResolver; import com.google.gerrit.server.logging.Metadata; import com.google.gerrit.server.logging.TraceContext; @@ -28,6 +29,7 @@ import com.google.inject.Singleton; import java.io.IOException; import java.util.Collection; import java.util.List; +import java.util.Optional; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; @@ -55,10 +57,20 @@ public class Reachable { */ public boolean fromRefs( Project.NameKey project, Repository repo, RevCommit commit, List refs) { + return fromRefs(project, repo, commit, refs, Optional.empty()); + } + + boolean fromRefs( + Project.NameKey project, + Repository repo, + RevCommit commit, + List refs, + Optional optionalUserProvider) { try (RevWalk rw = new RevWalk(repo)) { Collection filtered = - permissionBackend - .currentUser() + optionalUserProvider + .map(permissionBackend::user) + .orElse(permissionBackend.currentUser()) .project(project) .filter(refs, repo, RefFilterOptions.defaults()); diff --git a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java index 2e5c33b0fa..077131b0a3 100644 --- a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java +++ b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java @@ -30,12 +30,17 @@ import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectState; import com.google.inject.Inject; import com.google.inject.Provider; +import com.google.inject.assistedinject.Assisted; import java.util.Optional; import org.eclipse.jgit.errors.RepositoryNotFoundException; public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + public interface Factory { + ChangeIsVisibleToPredicate forUser(CurrentUser user); + } + protected final ChangeNotes.Factory notesFactory; protected final CurrentUser user; protected final PermissionBackend permissionBackend; @@ -45,10 +50,10 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate @Inject public ChangeIsVisibleToPredicate( ChangeNotes.Factory notesFactory, - CurrentUser user, PermissionBackend permissionBackend, ProjectCache projectCache, - Provider anonymousUserProvider) { + Provider anonymousUserProvider, + @Assisted CurrentUser user) { super(ChangeQueryBuilder.FIELD_VISIBLETO, IndexUtils.describe(user)); this.notesFactory = notesFactory; this.user = user; diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index f5cb458e4c..400db67a12 100644 --- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -49,7 +49,6 @@ import com.google.gerrit.index.query.QueryBuilder; import com.google.gerrit.index.query.QueryParseException; import com.google.gerrit.index.query.QueryRequiresAuthException; import com.google.gerrit.mail.Address; -import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.CommentsUtil; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; @@ -75,7 +74,6 @@ import com.google.gerrit.server.index.change.ChangeField; import com.google.gerrit.server.index.change.ChangeIndex; import com.google.gerrit.server.index.change.ChangeIndexCollection; import com.google.gerrit.server.index.change.ChangeIndexRewriter; -import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ReviewerStateInternal; import com.google.gerrit.server.patch.PatchListCache; import com.google.gerrit.server.permissions.PermissionBackend; @@ -217,7 +215,6 @@ public class ChangeQueryBuilder extends QueryBuilder hasOperands; @@ -233,7 +230,7 @@ public class ChangeQueryBuilder extends QueryBuilder anonymousUserProvider; + final ChangeIsVisibleToPredicate.Factory changeIsVisbleToPredicateFactory; final OperatorAliasConfig operatorAliasConfig; final boolean indexMergeable; final HasOperandAliasConfig hasOperandAliasConfig; @@ -250,7 +247,6 @@ public class ChangeQueryBuilder extends QueryBuilder self, PermissionBackend permissionBackend, - ChangeNotes.Factory notesFactory, ChangeData.Factory changeDataFactory, CommentsUtil commentsUtil, AccountResolver accountResolver, @@ -268,10 +264,10 @@ public class ChangeQueryBuilder extends QueryBuilder anonymousUserProvider, OperatorAliasConfig operatorAliasConfig, @GerritServerConfig Config gerritConfig, - HasOperandAliasConfig hasOperandAliasConfig) { + HasOperandAliasConfig hasOperandAliasConfig, + ChangeIsVisibleToPredicate.Factory changeIsVisbleToPredicateFactory) { this( queryProvider, rewriter, @@ -280,7 +276,6 @@ public class ChangeQueryBuilder extends QueryBuilder self, PermissionBackend permissionBackend, - ChangeNotes.Factory notesFactory, ChangeData.Factory changeDataFactory, CommentsUtil commentsUtil, AccountResolver accountResolver, @@ -330,17 +324,16 @@ public class ChangeQueryBuilder extends QueryBuilder anonymousUserProvider, OperatorAliasConfig operatorAliasConfig, boolean indexMergeable, - HasOperandAliasConfig hasOperandAliasConfig) { + HasOperandAliasConfig hasOperandAliasConfig, + ChangeIsVisibleToPredicate.Factory changeIsVisbleToPredicateFactory) { this.queryProvider = queryProvider; this.rewriter = rewriter; this.opFactories = opFactories; this.userFactory = userFactory; this.self = self; this.permissionBackend = permissionBackend; - this.notesFactory = notesFactory; this.changeDataFactory = changeDataFactory; this.commentsUtil = commentsUtil; this.accountResolver = accountResolver; @@ -359,7 +352,7 @@ public class ChangeQueryBuilder extends QueryBuilder visibleto(CurrentUser user) { - return new ChangeIsVisibleToPredicate( - args.notesFactory, - user, - args.permissionBackend, - args.projectCache, - args.anonymousUserProvider); + return args.changeIsVisbleToPredicateFactory.forUser(user); } public Predicate isVisible() throws QueryParseException { diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java index f9263a9922..40c04773ed 100644 --- a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java +++ b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java @@ -28,7 +28,6 @@ import com.google.gerrit.index.query.IndexPredicate; import com.google.gerrit.index.query.Predicate; import com.google.gerrit.index.query.QueryProcessor; import com.google.gerrit.metrics.MetricMaker; -import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.DynamicOptions; import com.google.gerrit.server.DynamicOptions.DynamicBean; @@ -40,9 +39,6 @@ import com.google.gerrit.server.index.change.ChangeIndexCollection; import com.google.gerrit.server.index.change.ChangeIndexRewriter; import com.google.gerrit.server.index.change.ChangeSchemaDefinitions; import com.google.gerrit.server.index.change.IndexedChangeQuery; -import com.google.gerrit.server.notedb.ChangeNotes; -import com.google.gerrit.server.permissions.PermissionBackend; -import com.google.gerrit.server.project.ProjectCache; import com.google.inject.Inject; import com.google.inject.Provider; import java.util.HashMap; @@ -58,11 +54,8 @@ import java.util.Set; public class ChangeQueryProcessor extends QueryProcessor implements DynamicOptions.BeanReceiver, DynamicOptions.BeanProvider { private final Provider userProvider; - private final ChangeNotes.Factory notesFactory; private final ImmutableListMultimap attributeFactoriesByPlugin; - private final PermissionBackend permissionBackend; - private final ProjectCache projectCache; - private final Provider anonymousUserProvider; + private final ChangeIsVisibleToPredicate.Factory changeIsVisibleToPredicateFactory; private final Map dynamicBeans = new HashMap<>(); static { @@ -80,11 +73,8 @@ public class ChangeQueryProcessor extends QueryProcessor IndexConfig indexConfig, ChangeIndexCollection indexes, ChangeIndexRewriter rewriter, - ChangeNotes.Factory notesFactory, DynamicSet attributeFactories, - PermissionBackend permissionBackend, - ProjectCache projectCache, - Provider anonymousUserProvider) { + ChangeIsVisibleToPredicate.Factory changeIsVisibleToPredicateFactory) { super( metricMaker, ChangeSchemaDefinitions.INSTANCE, @@ -94,10 +84,7 @@ public class ChangeQueryProcessor extends QueryProcessor FIELD_LIMIT, () -> limitsFactory.create(userProvider.get()).getQueryLimit()); this.userProvider = userProvider; - this.notesFactory = notesFactory; - this.permissionBackend = permissionBackend; - this.projectCache = projectCache; - this.anonymousUserProvider = anonymousUserProvider; + this.changeIsVisibleToPredicateFactory = changeIsVisibleToPredicateFactory; ImmutableListMultimap.Builder factoriesBuilder = ImmutableListMultimap.builder(); @@ -144,14 +131,7 @@ public class ChangeQueryProcessor extends QueryProcessor @Override protected Predicate enforceVisibility(Predicate pred) { return new AndChangeSource( - pred, - new ChangeIsVisibleToPredicate( - notesFactory, - userProvider.get(), - permissionBackend, - projectCache, - anonymousUserProvider), - start); + pred, changeIsVisibleToPredicateFactory.forUser(userProvider.get()), start); } @Override diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java index 2037049a84..7008bb9019 100644 --- a/java/com/google/gerrit/server/restapi/change/PostReview.java +++ b/java/com/google/gerrit/server/restapi/change/PostReview.java @@ -91,6 +91,7 @@ import com.google.gerrit.server.PublishCommentUtil; import com.google.gerrit.server.ReviewerSet; import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.change.AddReviewersEmail; +import com.google.gerrit.server.change.AddReviewersOp.Result; import com.google.gerrit.server.change.ChangeResource; import com.google.gerrit.server.change.EmailReviewComments; import com.google.gerrit.server.change.NotifyResolver; @@ -426,10 +427,15 @@ public class PostReview implements RestModifyView List
toByEmail = new ArrayList<>(); List
ccByEmail = new ArrayList<>(); for (ReviewerAddition addition : reviewerAdditions) { - if (addition.state() == ReviewerState.REVIEWER) { + Result reviewAdditionResult = addition.op.getResult(); + if (addition.state() == ReviewerState.REVIEWER + && (!reviewAdditionResult.addedReviewers().isEmpty() + || !reviewAdditionResult.addedReviewersByEmail().isEmpty())) { to.addAll(addition.reviewers); toByEmail.addAll(addition.reviewersByEmail); - } else if (addition.state() == ReviewerState.CC) { + } else if (addition.state() == ReviewerState.CC + && (!reviewAdditionResult.addedCCs().isEmpty() + || !reviewAdditionResult.addedCCsByEmail().isEmpty())) { cc.addAll(addition.reviewers); ccByEmail.addAll(addition.reviewersByEmail); } diff --git a/java/com/google/gerrit/server/restapi/project/CreateTag.java b/java/com/google/gerrit/server/restapi/project/CreateTag.java index b087f153bf..5cfb118d28 100644 --- a/java/com/google/gerrit/server/restapi/project/CreateTag.java +++ b/java/com/google/gerrit/server/restapi/project/CreateTag.java @@ -108,8 +108,11 @@ public class CreateTag implements RestCollectionCreateView> queryCache; private final Map> heads; private final ProjectCache projectCache; - private final ChangeIsVisibleToPredicate changeIsVisibleToPredicate; + private final ChangeIsVisibleToPredicate.Factory changeIsVisibleToPredicateFactory; @Inject LocalMergeSuperSetComputation( PermissionBackend permissionBackend, Provider queryProvider, ProjectCache projectCache, - ChangeIsVisibleToPredicate changeIsVisibleToPredicate) { + ChangeIsVisibleToPredicate.Factory changeIsVisibleToPredicateFactory) { this.projectCache = projectCache; this.permissionBackend = permissionBackend; this.queryProvider = queryProvider; this.queryCache = new HashMap<>(); this.heads = new HashMap<>(); - this.changeIsVisibleToPredicate = changeIsVisibleToPredicate; + this.changeIsVisibleToPredicateFactory = changeIsVisibleToPredicateFactory; } @Override @@ -150,7 +150,8 @@ public class LocalMergeSuperSetComputation implements MergeSuperSetComputation { walkChangesByHashes(visibleCommits, Collections.emptySet(), or, b); Set nonVisibleHashes = walkChangesByHashes(nonVisibleCommits, visibleHashes, or, b); - ChangeSet partialSet = byCommitsOnBranchNotMerged(or, b, visibleHashes, nonVisibleHashes); + ChangeSet partialSet = + byCommitsOnBranchNotMerged(or, b, visibleHashes, nonVisibleHashes, user); Iterables.addAll(visibleChanges, partialSet.changes()); Iterables.addAll(nonVisibleChanges, partialSet.nonVisibleChanges()); } @@ -207,13 +208,19 @@ public class LocalMergeSuperSetComputation implements MergeSuperSetComputation { } private ChangeSet byCommitsOnBranchNotMerged( - OpenRepo or, BranchNameKey branch, Set visibleHashes, Set nonVisibleHashes) + OpenRepo or, + BranchNameKey branch, + Set visibleHashes, + Set nonVisibleHashes, + CurrentUser user) throws IOException { List potentiallyVisibleChanges = byCommitsOnBranchNotMerged(or, branch, visibleHashes); List invisibleChanges = new ArrayList<>(byCommitsOnBranchNotMerged(or, branch, nonVisibleHashes)); List visibleChanges = new ArrayList<>(potentiallyVisibleChanges.size()); + ChangeIsVisibleToPredicate changeIsVisibleToPredicate = + changeIsVisibleToPredicateFactory.forUser(user); for (ChangeData cd : potentiallyVisibleChanges) { if (changeIsVisibleToPredicate.match(cd)) { visibleChanges.add(cd); diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java index b8d20ab0c6..47508bb8ac 100644 --- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java @@ -90,6 +90,7 @@ import com.google.gerrit.extensions.api.accounts.DeletedDraftCommentInfo; import com.google.gerrit.extensions.api.accounts.EmailInput; import com.google.gerrit.extensions.api.changes.AddReviewerInput; import com.google.gerrit.extensions.api.changes.DraftInput; +import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.api.changes.StarsInput; import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo; import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo; @@ -883,6 +884,99 @@ public class AccountIT extends AbstractDaemonTest { } } + @Test + public void addExistingReviewersUsingPostReview() throws Exception { + PushOneCommit.Result r = createChange(); + + // First reviewer added to the change + ReviewInput input = new ReviewInput(); + input.reviewers = new ArrayList<>(1); + AddReviewerInput addReviewerInput = new AddReviewerInput(); + addReviewerInput.reviewer = user.email(); + input.reviewers.add(addReviewerInput); + gApi.changes().id(r.getChangeId()).current().review(input); + List messages = sender.getMessages(); + assertThat(messages).hasSize(1); + Message message = messages.get(0); + assertThat(message.rcpt()).containsExactly(user.getNameEmail()); + assertMailReplyTo(message, admin.email()); + + sender.clear(); + + // Second reviewer and existing reviewer added to the change + ReviewInput input2 = new ReviewInput(); + input2.reviewers = new ArrayList<>(2); + AddReviewerInput addReviewerInput2 = new AddReviewerInput(); + addReviewerInput2.reviewer = user.email(); + input2.reviewers.add(addReviewerInput2); + AddReviewerInput addReviewerInput3 = new AddReviewerInput(); + + TestAccount user2 = accountCreator.user2(); + addReviewerInput3.reviewer = user2.email(); + input2.reviewers.add(addReviewerInput3); + + gApi.changes().id(r.getChangeId()).current().review(input2); + List messages2 = sender.getMessages(); + assertThat(messages2).hasSize(1); + Message message2 = messages2.get(0); + assertThat(message2.rcpt()).containsExactly(user.getNameEmail(), user2.getNameEmail()); + assertMailReplyTo(message, admin.email()); + + sender.clear(); + + // Existing reviewers re-added to the change: no notifications + ReviewInput input3 = new ReviewInput(); + input3.reviewers = new ArrayList<>(2); + AddReviewerInput addReviewerInput4 = new AddReviewerInput(); + addReviewerInput4.reviewer = user.email(); + input3.reviewers.add(addReviewerInput4); + AddReviewerInput addReviewerInput5 = new AddReviewerInput(); + + addReviewerInput5.reviewer = user2.email(); + input3.reviewers.add(addReviewerInput5); + + gApi.changes().id(r.getChangeId()).current().review(input3); + List messages3 = sender.getMessages(); + assertThat(messages3).isEmpty(); + } + + @Test + public void addExistingReviewersUsingAddReviewer() throws Exception { + PushOneCommit.Result r = createChange(); + + // First reviewer added to the change + AddReviewerInput addReviewerInput = new AddReviewerInput(); + addReviewerInput.reviewer = user.email(); + gApi.changes().id(r.getChangeId()).addReviewer(addReviewerInput); + List messages = sender.getMessages(); + assertThat(messages).hasSize(1); + Message message = messages.get(0); + assertThat(message.rcpt()).containsExactly(user.getNameEmail()); + assertMailReplyTo(message, admin.email()); + + sender.clear(); + + // Second reviewer added to the change + TestAccount user2 = accountCreator.user2(); + AddReviewerInput addReviewerInput2 = new AddReviewerInput(); + addReviewerInput2.reviewer = user2.email(); + gApi.changes().id(r.getChangeId()).addReviewer(addReviewerInput2); + List messages2 = sender.getMessages(); + assertThat(messages2).hasSize(1); + Message message2 = messages2.get(0); + assertThat(message2.rcpt()).containsExactly(user.getNameEmail(), user2.getNameEmail()); + assertMailReplyTo(message2, admin.email()); + + sender.clear(); + + // Exiting reviewer re-added to the change: no notifications + AddReviewerInput addReviewerInput3 = new AddReviewerInput(); + addReviewerInput3.reviewer = user2.email(); + gApi.changes().id(r.getChangeId()).addReviewer(addReviewerInput3); + List messages3 = sender.getMessages(); + assertThat(messages3).isEmpty(); + } + @Test public void suggestAccounts() throws Exception { AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter(); diff --git a/javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java similarity index 99% rename from javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java rename to javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java index d3c085588a..dcee118d5f 100644 --- a/javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java +++ b/javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java @@ -22,7 +22,6 @@ import static java.util.stream.Collectors.toList; import com.google.common.collect.Iterables; import com.google.gerrit.acceptance.AbstractDaemonTest; -import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.TestAccount; import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; @@ -53,13 +52,18 @@ import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.RefSpec; import org.eclipse.jgit.transport.RemoteRefUpdate; +import org.junit.Before; import org.junit.Test; -@NoHttpd -public class SubmitOnPushIT extends AbstractDaemonTest { +public abstract class AbstractSubmitOnPush extends AbstractDaemonTest { @Inject private ApprovalsUtil approvalsUtil; @Inject private ProjectOperations projectOperations; + @Before + public void blockAnonymous() throws Exception { + blockAnonymousRead(); + } + @Test public void submitOnPush() throws Exception { projectOperations diff --git a/javatests/com/google/gerrit/acceptance/git/HttpSubmitOnPushIT.java b/javatests/com/google/gerrit/acceptance/git/HttpSubmitOnPushIT.java new file mode 100644 index 0000000000..373341536f --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/git/HttpSubmitOnPushIT.java @@ -0,0 +1,30 @@ +// Copyright (C) 2020 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.acceptance.git; + +import com.google.gerrit.acceptance.GitUtil; +import org.eclipse.jgit.transport.CredentialsProvider; +import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider; +import org.junit.Before; + +public class HttpSubmitOnPushIT extends AbstractSubmitOnPush { + + @Before + public void cloneProjectOverHttp() throws Exception { + CredentialsProvider.setDefault( + new UsernamePasswordCredentialsProvider(admin.username(), admin.httpPassword())); + testRepo = GitUtil.cloneProject(project, admin.getHttpUrl(server) + "/a/" + project.get()); + } +} diff --git a/javatests/com/google/gerrit/acceptance/git/SshSubmitOnPushIT.java b/javatests/com/google/gerrit/acceptance/git/SshSubmitOnPushIT.java new file mode 100644 index 0000000000..3a18257f15 --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/git/SshSubmitOnPushIT.java @@ -0,0 +1,27 @@ +// Copyright (C) 2020 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.acceptance.git; + +import com.google.gerrit.acceptance.NoHttpd; +import org.junit.Before; + +@NoHttpd +public class SshSubmitOnPushIT extends AbstractSubmitOnPush { + + @Before + public void cloneProjectOverSsh() throws Exception { + testRepo = cloneProject(project, admin); + } +} diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AbstractHttpPushTag.java b/javatests/com/google/gerrit/acceptance/rest/project/AbstractHttpPushTag.java new file mode 100644 index 0000000000..5e2af147d8 --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/rest/project/AbstractHttpPushTag.java @@ -0,0 +1,31 @@ +// Copyright (C) 2020 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.acceptance.rest.project; + +import com.google.gerrit.acceptance.GitUtil; +import org.eclipse.jgit.transport.CredentialsProvider; +import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider; +import org.junit.Before; + +public abstract class AbstractHttpPushTag extends AbstractPushTag { + + @Before + public void cloneProjectOverHttp() throws Exception { + // clone with user to avoid inherited tag permissions of admin user + CredentialsProvider.setDefault( + new UsernamePasswordCredentialsProvider(user.username(), user.httpPassword())); + testRepo = GitUtil.cloneProject(project, user.getHttpUrl(server) + "/" + project.get()); + } +} diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java b/javatests/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java index 79494c5354..d70d120c19 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java @@ -28,7 +28,6 @@ import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS import com.google.common.base.MoreObjects; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GitUtil; -import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; import com.google.gerrit.common.data.Permission; import com.google.gerrit.entities.RefNames; @@ -41,7 +40,6 @@ import org.eclipse.jgit.transport.RemoteRefUpdate.Status; import org.junit.Before; import org.junit.Test; -@NoHttpd public abstract class AbstractPushTag extends AbstractDaemonTest { enum TagType { LIGHTWEIGHT(Permission.CREATE), @@ -61,11 +59,9 @@ public abstract class AbstractPushTag extends AbstractDaemonTest { @Before public void setUpTestEnvironment() throws Exception { - // clone with user to avoid inherited tag permissions of admin user - testRepo = cloneProject(project, user); - initialHead = projectOperations.project(project).getHead("master"); tagType = getTagType(); + blockAnonymousRead(); } protected abstract TagType getTagType(); diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AbstractSshPushTag.java b/javatests/com/google/gerrit/acceptance/rest/project/AbstractSshPushTag.java new file mode 100644 index 0000000000..35297ae7d4 --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/rest/project/AbstractSshPushTag.java @@ -0,0 +1,29 @@ +// Copyright (C) 2020 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.acceptance.rest.project; + +import com.google.gerrit.acceptance.NoHttpd; +import com.google.gerrit.acceptance.UseSsh; +import org.junit.Before; + +@NoHttpd +@UseSsh +public abstract class AbstractSshPushTag extends AbstractPushTag { + @Before + public void cloneProjectOverSsh() throws Exception { + // clone with user to avoid inherited tag permissions of admin user + testRepo = cloneProject(project, user); + } +} diff --git a/javatests/com/google/gerrit/acceptance/rest/project/BUILD b/javatests/com/google/gerrit/acceptance/rest/project/BUILD index b50a12b1b5..54ae5af44f 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/BUILD +++ b/javatests/com/google/gerrit/acceptance/rest/project/BUILD @@ -57,7 +57,9 @@ java_library( name = "push_tag_util", testonly = True, srcs = [ + "AbstractHttpPushTag.java", "AbstractPushTag.java", + "AbstractSshPushTag.java", ], deps = [ "//java/com/google/gerrit/acceptance:lib", diff --git a/javatests/com/google/gerrit/acceptance/rest/project/HttpPushAnnotatedTagIT.java b/javatests/com/google/gerrit/acceptance/rest/project/HttpPushAnnotatedTagIT.java new file mode 100644 index 0000000000..bb08f41bd9 --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/rest/project/HttpPushAnnotatedTagIT.java @@ -0,0 +1,23 @@ +// Copyright (C) 2020 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.acceptance.rest.project; + +public class HttpPushAnnotatedTagIT extends AbstractHttpPushTag { + + @Override + protected TagType getTagType() { + return TagType.ANNOTATED; + } +} diff --git a/javatests/com/google/gerrit/acceptance/rest/project/PushLightweightTagIT.java b/javatests/com/google/gerrit/acceptance/rest/project/HttpPushLightweightTagIT.java similarity index 85% rename from javatests/com/google/gerrit/acceptance/rest/project/PushLightweightTagIT.java rename to javatests/com/google/gerrit/acceptance/rest/project/HttpPushLightweightTagIT.java index 20d83a0cd2..c89c33247c 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/PushLightweightTagIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/HttpPushLightweightTagIT.java @@ -1,4 +1,4 @@ -// Copyright (C) 2017 The Android Open Source Project +// Copyright (C) 2020 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. @@ -14,7 +14,7 @@ package com.google.gerrit.acceptance.rest.project; -public class PushLightweightTagIT extends AbstractPushTag { +public class HttpPushLightweightTagIT extends AbstractHttpPushTag { @Override protected TagType getTagType() { diff --git a/javatests/com/google/gerrit/acceptance/rest/project/PushAnnotatedTagIT.java b/javatests/com/google/gerrit/acceptance/rest/project/SshPushAnnotatedTagIT.java similarity index 85% rename from javatests/com/google/gerrit/acceptance/rest/project/PushAnnotatedTagIT.java rename to javatests/com/google/gerrit/acceptance/rest/project/SshPushAnnotatedTagIT.java index 24c8ed0f85..df7636a33c 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/PushAnnotatedTagIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/SshPushAnnotatedTagIT.java @@ -1,4 +1,4 @@ -// Copyright (C) 2017 The Android Open Source Project +// Copyright (C) 2020 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. @@ -14,7 +14,7 @@ package com.google.gerrit.acceptance.rest.project; -public class PushAnnotatedTagIT extends AbstractPushTag { +public class SshPushAnnotatedTagIT extends AbstractSshPushTag { @Override protected TagType getTagType() { diff --git a/javatests/com/google/gerrit/acceptance/rest/project/SshPushLightweightTagIT.java b/javatests/com/google/gerrit/acceptance/rest/project/SshPushLightweightTagIT.java new file mode 100644 index 0000000000..bcf7eb9b5f --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/rest/project/SshPushLightweightTagIT.java @@ -0,0 +1,28 @@ +// Copyright (C) 2020 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.acceptance.rest.project; + +import com.google.gerrit.acceptance.NoHttpd; +import com.google.gerrit.acceptance.UseSsh; + +@NoHttpd +@UseSsh +public class SshPushLightweightTagIT extends AbstractSshPushTag { + + @Override + protected TagType getTagType() { + return TagType.LIGHTWEIGHT; + } +} diff --git a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java index 3becb8197e..f5d2db464e 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java @@ -38,10 +38,13 @@ import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.MethodNotAllowedException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; +import com.google.gerrit.server.project.ProjectConfig; import com.google.inject.Inject; import java.sql.Timestamp; +import java.util.Arrays; import java.util.List; import org.eclipse.jgit.revwalk.RevCommit; +import org.junit.Before; import org.junit.Test; @NoHttpd @@ -66,6 +69,16 @@ public class TagsIT extends AbstractDaemonTest { @Inject private ProjectOperations projectOperations; @Inject private RequestScopeOperations requestScopeOperations; + @Before + public void setupPermissions() throws Exception { + try (ProjectConfigUpdate u = updateProject(allProjects)) { + ProjectConfig cfg = u.getConfig(); + removeAllBranchPermissions( + cfg, Permission.CREATE, Permission.CREATE_TAG, Permission.CREATE_SIGNED_TAG); + u.save(); + } + } + @Test public void listTagsOfNonExistingProject() throws Exception { assertThrows( @@ -455,4 +468,10 @@ public class TagsIT extends AbstractDaemonTest { .add(allow(Permission.CREATE_SIGNED_TAG).ref(R_TAGS + "*").group(adminGroupUuid())) .update(); } + + private static void removeAllBranchPermissions(ProjectConfig cfg, String... permissions) { + cfg.getAccessSections().stream() + .filter(s -> s.getName().startsWith("refs/tags/")) + .forEach(s -> Arrays.stream(permissions).forEach(s::removePermission)); + } } diff --git a/javatests/com/google/gerrit/acceptance/server/httpd/BUILD b/javatests/com/google/gerrit/acceptance/server/httpd/BUILD new file mode 100644 index 0000000000..d1a64c01ed --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/server/httpd/BUILD @@ -0,0 +1,7 @@ +load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests") + +acceptance_tests( + srcs = glob(["*IT.java"]), + group = "server_httpd", + labels = ["server"], +) diff --git a/javatests/com/google/gerrit/acceptance/server/httpd/HttpLogoutServletIT.java b/javatests/com/google/gerrit/acceptance/server/httpd/HttpLogoutServletIT.java new file mode 100644 index 0000000000..1dea800849 --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/server/httpd/HttpLogoutServletIT.java @@ -0,0 +1,112 @@ +// Copyright (C) 2020 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.acceptance.server.httpd; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.gerrit.acceptance.StandaloneSiteTest; +import com.google.gerrit.server.config.GerritServerConfig; +import com.google.gerrit.testing.ConfigSuite; +import com.google.inject.Inject; +import java.io.IOException; +import java.net.InetAddress; +import java.net.ServerSocket; +import java.net.URISyntaxException; +import java.util.Arrays; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; +import org.apache.http.HttpHost; +import org.apache.http.HttpResponse; +import org.apache.http.HttpStatus; +import org.apache.http.client.HttpClient; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.HttpClientBuilder; +import org.eclipse.jgit.lib.Config; +import org.eclipse.jgit.transport.URIish; +import org.junit.Before; +import org.junit.Test; + +public class HttpLogoutServletIT extends StandaloneSiteTest { + private static final String LOCALHOST = InetAddress.getLoopbackAddress().getHostName(); + + @ConfigSuite.Config + public static Config secondConfig() throws IOException { + Config cfg = new Config(); + cfg.setString("auth", null, "logouturl", "/test-logout"); + cfg.setString("gerrit", null, "canonicalWebUrl", "https://" + LOCALHOST + ":8443/"); + cfg.setString("httpd", null, "listenUrl", "proxy-https://" + LOCALHOST + ":" + getFreePort()); + return cfg; + } + + @Inject @GerritServerConfig private Config gerritConfig; + + private HttpClient httpClient; + + @Before + public void setUp() { + httpClient = HttpClientBuilder.create().disableRedirectHandling().build(); + } + + @Test + public void shouldHonourCanonicalWebUrlProxyWhenRedirectAfterLogout() throws Exception { + try (ServerContext ctx = startServer()) { + ctx.getInjector().injectMembers(this); + + URIish listenUrl = new URIish(gerritConfig.getString("httpd", null, "listenUrl")); + URIish canonicalWebUrl = + new URIish(gerritConfig.getString("gerrit", null, "canonicalWebUrl")); + + String logoutPath = + Optional.ofNullable(baseConfig.getString("auth", null, "logouturl")).orElse("/"); + + HttpGet getLogout = new HttpGet("/logout"); + getLogout.addHeader("X-Forwarded-Host", canonicalWebUrl.getHost()); + getLogout.addHeader("X-Forwarded-Port", "" + canonicalWebUrl.getPort()); + getLogout.addHeader("X-Forwarded-Proto", canonicalWebUrl.getScheme()); + + HttpResponse logoutResponse = + httpClient.execute(new HttpHost(listenUrl.getHost(), listenUrl.getPort()), getLogout); + + assertThat(logoutResponse.getStatusLine().getStatusCode()) + .isEqualTo(HttpStatus.SC_MOVED_TEMPORARILY); + assertThat(getLocationHeaderURIish(logoutResponse)) + .containsExactly(canonicalWebUrl.setPath(logoutPath)); + } + } + + private List getLocationHeaderURIish(HttpResponse logoutResponse) { + return Arrays.stream(logoutResponse.getHeaders("Location")) + .map(h -> h.getValue()) + .map(HttpLogoutServletIT::unsafeNewURIish) + .filter(u -> u.isPresent()) + .map(u -> u.get()) + .collect(Collectors.toList()); + } + + private static Optional unsafeNewURIish(String uri) { + try { + return Optional.of(new URIish(uri)); + } catch (URISyntaxException e) { + return Optional.empty(); + } + } + + private static int getFreePort() throws IOException { + try (ServerSocket s = new ServerSocket(0)) { + return s.getLocalPort(); + } + } +} diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java index 879c7c5c3c..e4b45edbab 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java @@ -38,12 +38,6 @@ public class ElasticContainer extends ElasticsearchContainer { private static String getImageName(ElasticVersion version) { switch (version) { - case V6_2: - return "blacktop/elasticsearch:6.2.4"; - case V6_3: - return "blacktop/elasticsearch:6.3.2"; - case V6_4: - return "blacktop/elasticsearch:6.4.3"; case V6_5: return "blacktop/elasticsearch:6.5.4"; case V6_6: diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java index 20c8a17130..e36ff0be45 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java @@ -22,15 +22,6 @@ import org.junit.Test; public class ElasticVersionTest { @Test public void supportedVersion() throws Exception { - assertThat(ElasticVersion.forVersion("6.2.0")).isEqualTo(ElasticVersion.V6_2); - assertThat(ElasticVersion.forVersion("6.2.4")).isEqualTo(ElasticVersion.V6_2); - - assertThat(ElasticVersion.forVersion("6.3.0")).isEqualTo(ElasticVersion.V6_3); - assertThat(ElasticVersion.forVersion("6.3.2")).isEqualTo(ElasticVersion.V6_3); - - assertThat(ElasticVersion.forVersion("6.4.0")).isEqualTo(ElasticVersion.V6_4); - assertThat(ElasticVersion.forVersion("6.4.1")).isEqualTo(ElasticVersion.V6_4); - assertThat(ElasticVersion.forVersion("6.5.0")).isEqualTo(ElasticVersion.V6_5); assertThat(ElasticVersion.forVersion("6.5.1")).isEqualTo(ElasticVersion.V6_5); @@ -79,9 +70,6 @@ public class ElasticVersionTest { @Test public void atLeastMinorVersion() throws Exception { - assertThat(ElasticVersion.V6_2.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); - assertThat(ElasticVersion.V6_3.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); - assertThat(ElasticVersion.V6_4.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); assertThat(ElasticVersion.V6_5.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); assertThat(ElasticVersion.V6_6.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); assertThat(ElasticVersion.V6_7.isAtLeastMinorVersion(ElasticVersion.V6_7)).isTrue(); @@ -97,9 +85,6 @@ public class ElasticVersionTest { @Test public void version6OrLater() throws Exception { - assertThat(ElasticVersion.V6_2.isV6OrLater()).isTrue(); - assertThat(ElasticVersion.V6_3.isV6OrLater()).isTrue(); - assertThat(ElasticVersion.V6_4.isV6OrLater()).isTrue(); assertThat(ElasticVersion.V6_5.isV6OrLater()).isTrue(); assertThat(ElasticVersion.V6_6.isV6OrLater()).isTrue(); assertThat(ElasticVersion.V6_7.isV6OrLater()).isTrue(); @@ -115,9 +100,6 @@ public class ElasticVersionTest { @Test public void version7OrLater() throws Exception { - assertThat(ElasticVersion.V6_2.isV7OrLater()).isFalse(); - assertThat(ElasticVersion.V6_3.isV7OrLater()).isFalse(); - assertThat(ElasticVersion.V6_4.isV7OrLater()).isFalse(); assertThat(ElasticVersion.V6_5.isV7OrLater()).isFalse(); assertThat(ElasticVersion.V6_6.isV7OrLater()).isFalse(); assertThat(ElasticVersion.V6_7.isV7OrLater()).isFalse(); diff --git a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java index 7f133cea3d..a936d28555 100644 --- a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java +++ b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java @@ -44,7 +44,6 @@ public class FakeQueryBuilder extends ChangeQueryBuilder { null, null, null, - null, indexes, null, null, @@ -53,8 +52,8 @@ public class FakeQueryBuilder extends ChangeQueryBuilder { null, null, null, - null, new Config(), + null, null)); } diff --git a/lib/LICENSE-PublicDomain b/lib/LICENSE-PublicDomain deleted file mode 100644 index 8a71ce09b0..0000000000 --- a/lib/LICENSE-PublicDomain +++ /dev/null @@ -1 +0,0 @@ -This software has been placed in the public domain by its author(s). diff --git a/lib/guice/BUILD b/lib/guice/BUILD index f73984b7c8..14179d6620 100644 --- a/lib/guice/BUILD +++ b/lib/guice/BUILD @@ -1,4 +1,9 @@ -load("@rules_java//java:defs.bzl", "java_library") +load("@rules_java//java:defs.bzl", "java_import", "java_library") + +java_import( + name = "guice-library-no-aop", + jars = ["@guice-library-no-aop//file"], +) java_library( name = "guice", @@ -14,8 +19,7 @@ java_library( name = "guice-library", data = ["//lib:LICENSE-Apache2.0"], visibility = ["//visibility:public"], - exports = ["@guice-library//jar"], - runtime_deps = ["aopalliance"], + exports = [":guice-library-no-aop"], ) java_library( @@ -34,12 +38,6 @@ java_library( runtime_deps = [":guice"], ) -java_library( - name = "aopalliance", - data = ["//lib:LICENSE-PublicDomain"], - exports = ["@aopalliance//jar"], -) - java_library( name = "javax_inject", data = ["//lib:LICENSE-Apache2.0"], diff --git a/resources/com/google/gerrit/server/commit-msg_test.sh b/resources/com/google/gerrit/server/commit-msg_test.sh index c016a8925b..d797be30fd 100755 --- a/resources/com/google/gerrit/server/commit-msg_test.sh +++ b/resources/com/google/gerrit/server/commit-msg_test.sh @@ -63,9 +63,9 @@ index 625fd613d9..03aeba3b21 100755 @@ -38,6 +38,7 @@ context line - + +hello, world - + context line EOF @@ -111,7 +111,7 @@ EOF } # Change-Id goes after existing trailers. -function test_at_start { +function test_at_end { cat << EOF > input bla bla @@ -119,16 +119,16 @@ Bug: #123 EOF ${hook} input || fail "failed hook execution" - result=$(git interpret-trailers --parse input | head -1 | grep ^Change-Id) + result=$(tail -1 input | grep ^Change-Id) if [[ -z "${result}" ]] ; then echo "after: " cat input - fail "did not find Change-Id at start" + fail "did not find Change-Id at end" fi } -function test_dash_at_start { +function test_dash_at_end { if [[ ! -x /bin/dash ]] ; then echo "/bin/dash not installed; skipping dash test." return @@ -142,12 +142,12 @@ EOF /bin/dash ${hook} input || fail "failed hook execution" - result=$(git interpret-trailers --parse input | head -1 | grep ^Change-Id) + result=$(tail -1 input | grep ^Change-Id) if [[ -z "${result}" ]] ; then echo "after: " cat input - fail "did not find Change-Id at start" + fail "did not find Change-Id at end" fi } diff --git a/resources/com/google/gerrit/server/tools/root/TOC b/resources/com/google/gerrit/server/tools/root/TOC index 647b5aad1f..7a48c646f1 100644 --- a/resources/com/google/gerrit/server/tools/root/TOC +++ b/resources/com/google/gerrit/server/tools/root/TOC @@ -5,4 +5,6 @@ 755 hooks/commit-msg +755 hooks/commit-msg-legacy + 755 scripts/reposize.sh diff --git a/resources/com/google/gerrit/server/tools/root/hooks/commit-msg b/resources/com/google/gerrit/server/tools/root/hooks/commit-msg index 2b1a2fc1c4..290123258b 100755 --- a/resources/com/google/gerrit/server/tools/root/hooks/commit-msg +++ b/resources/com/google/gerrit/server/tools/root/hooks/commit-msg @@ -50,7 +50,7 @@ fi # Avoid the --in-place option which only appeared in Git 2.8 # Avoid the --if-exists option which only appeared in Git 2.15 -if ! git -c trailer.ifexists=doNothing interpret-trailers --where start \ +if ! git -c trailer.ifexists=doNothing interpret-trailers \ --trailer "Change-Id: I${random}" < "$1" > "${dest}" ; then echo "cannot insert change-id line in $1" exit 1 diff --git a/resources/com/google/gerrit/server/tools/root/hooks/commit-msg-legacy b/resources/com/google/gerrit/server/tools/root/hooks/commit-msg-legacy new file mode 100755 index 0000000000..4c64559aeb --- /dev/null +++ b/resources/com/google/gerrit/server/tools/root/hooks/commit-msg-legacy @@ -0,0 +1,193 @@ +#!/bin/sh +# +# Part of Gerrit Code Review (https://www.gerritcodereview.com/) +# +# Copyright (C) 2009 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. +# + +unset GREP_OPTIONS + +CHANGE_ID_AFTER="Bug|Depends-On|Issue|Test|Feature|Fixes|Fixed" +MSG="$1" + +# Check for, and add if missing, a unique Change-Id +# +add_ChangeId() { + clean_message=`sed -e ' + /^diff --git .*/{ + s/// + q + } + /^Signed-off-by:/d + /^#/d + ' "$MSG" | git stripspace` + if test -z "$clean_message" + then + return + fi + + # Do not add Change-Id to temp commits + if echo "$clean_message" | head -1 | grep -q '^\(fixup\|squash\)!' + then + return + fi + + if test "false" = "`git config --bool --get gerrit.createChangeId`" + then + return + fi + + # Does Change-Id: already exist? if so, exit (no change). + if grep -i '^Change-Id:' "$MSG" >/dev/null + then + return + fi + + id=`_gen_ChangeId` + T="$MSG.tmp.$$" + AWK=awk + if [ -x /usr/xpg4/bin/awk ]; then + # Solaris AWK is just too broken + AWK=/usr/xpg4/bin/awk + fi + + # Get core.commentChar from git config or use default symbol + commentChar=`git config --get core.commentChar` + commentChar=${commentChar:-#} + + # How this works: + # - parse the commit message as (textLine+ blankLine*)* + # - assume textLine+ to be a footer until proven otherwise + # - exception: the first block is not footer (as it is the title) + # - read textLine+ into a variable + # - then count blankLines + # - once the next textLine appears, print textLine+ blankLine* as these + # aren't footer + # - in END, the last textLine+ block is available for footer parsing + $AWK ' + BEGIN { + if (match(ENVIRON["OS"], "Windows")) { + RS="\r?\n" # Required on recent Cygwin + } + # while we start with the assumption that textLine+ + # is a footer, the first block is not. + isFooter = 0 + footerComment = 0 + blankLines = 0 + } + + # Skip lines starting with commentChar without any spaces before it. + /^'"$commentChar"'/ { next } + + # Skip the line starting with the diff command and everything after it, + # up to the end of the file, assuming it is only patch data. + # If more than one line before the diff was empty, strip all but one. + /^diff --git / { + blankLines = 0 + while (getline) { } + next + } + + # Count blank lines outside footer comments + /^$/ && (footerComment == 0) { + blankLines++ + next + } + + # Catch footer comment + /^\[[a-zA-Z0-9-]+:/ && (isFooter == 1) { + footerComment = 1 + } + + /]$/ && (footerComment == 1) { + footerComment = 2 + } + + # We have a non-blank line after blank lines. Handle this. + (blankLines > 0) { + print lines + for (i = 0; i < blankLines; i++) { + print "" + } + + lines = "" + blankLines = 0 + isFooter = 1 + footerComment = 0 + } + + # Detect that the current block is not the footer + (footerComment == 0) && (!/^\[?[a-zA-Z0-9-]+:/ || /^[a-zA-Z0-9-]+:\/\//) { + isFooter = 0 + } + + { + # We need this information about the current last comment line + if (footerComment == 2) { + footerComment = 0 + } + if (lines != "") { + lines = lines "\n"; + } + lines = lines $0 + } + + # Footer handling: + # If the last block is considered a footer, splice in the Change-Id at the + # right place. + # Look for the right place to inject Change-Id by considering + # CHANGE_ID_AFTER. Keys listed in it (case insensitive) come first, + # then Change-Id, then everything else (eg. Signed-off-by:). + # + # Otherwise just print the last block, a new line and the Change-Id as a + # block of its own. + END { + unprinted = 1 + if (isFooter == 0) { + print lines "\n" + lines = "" + } + changeIdAfter = "^(" tolower("'"$CHANGE_ID_AFTER"'") "):" + numlines = split(lines, footer, "\n") + for (line = 1; line <= numlines; line++) { + if (unprinted && match(tolower(footer[line]), changeIdAfter) != 1) { + unprinted = 0 + print "Change-Id: I'"$id"'" + } + print footer[line] + } + if (unprinted) { + print "Change-Id: I'"$id"'" + } + }' "$MSG" > "$T" && mv "$T" "$MSG" || rm -f "$T" +} +_gen_ChangeIdInput() { + echo "tree `git write-tree`" + if parent=`git rev-parse "HEAD^0" 2>/dev/null` + then + echo "parent $parent" + fi + echo "author `git var GIT_AUTHOR_IDENT`" + echo "committer `git var GIT_COMMITTER_IDENT`" + echo + printf '%s' "$clean_message" +} +_gen_ChangeId() { + _gen_ChangeIdInput | + git hash-object -t commit --stdin +} + + +add_ChangeId diff --git a/tools/BUILD b/tools/BUILD index 7f890b1b52..5159177eef 100644 --- a/tools/BUILD +++ b/tools/BUILD @@ -70,6 +70,7 @@ java_package_configuration( "-Xep:NullableConstructor:ERROR", "-Xep:NullablePrimitive:ERROR", "-Xep:NullableVoid:ERROR", + "-Xep:ObjectToString:ERROR", "-Xep:OperatorPrecedence:ERROR", "-Xep:OverridesGuiceInjectableMethod:ERROR", "-Xep:PreconditionsInvalidPlaceholder:ERROR", diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl index 14b51095a5..7de7ec9fcd 100644 --- a/tools/nongoogle.bzl +++ b/tools/nongoogle.bzl @@ -120,18 +120,18 @@ def declare_nongoogle_deps(): sha1 = "dc13ae4faca6df981fc7aeb5a522d9db446d5d50", ) - TESTCONTAINERS_VERSION = "1.13.0" + TESTCONTAINERS_VERSION = "1.14.0" maven_jar( name = "testcontainers", artifact = "org.testcontainers:testcontainers:" + TESTCONTAINERS_VERSION, - sha1 = "c92d1094d2b227e881f66bf09872c46d91ce9ac5", + sha1 = "c0d6aea93f4f7ff4b0d559e31308340eaa398798", ) maven_jar( name = "testcontainers-elasticsearch", artifact = "org.testcontainers:elasticsearch:" + TESTCONTAINERS_VERSION, - sha1 = "09353bd960b3f0d26ba7652ae57bb4ac46d043a2", + sha1 = "6df7bc7cb5e99c6d9528ea28dd16dbb042b1beec", ) maven_jar(