-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lazily acquire JDK storage locations #2263
Changes from 6 commits
230615d
423b533
f768182
03b4139
3f0ee45
e7c3c18
3751c70
482bdc1
ae022f2
33a2bff
965e6ce
af3cbe9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,9 +16,11 @@ | |
|
||
package com.palantir.baseline.extensions; | ||
|
||
import com.palantir.baseline.plugins.javaversions.LazilyConfiguredMapping; | ||
import java.util.Optional; | ||
import java.util.concurrent.atomic.AtomicReference; | ||
import javax.inject.Inject; | ||
import org.gradle.api.Project; | ||
import org.gradle.api.provider.MapProperty; | ||
import org.gradle.api.provider.Property; | ||
import org.gradle.jvm.toolchain.JavaInstallationMetadata; | ||
import org.gradle.jvm.toolchain.JavaLanguageVersion; | ||
|
@@ -31,7 +33,8 @@ public class BaselineJavaVersionsExtension { | |
private final Property<JavaLanguageVersion> libraryTarget; | ||
private final Property<JavaLanguageVersion> distributionTarget; | ||
private final Property<JavaLanguageVersion> runtime; | ||
private final MapProperty<JavaLanguageVersion, JavaInstallationMetadata> jdks; | ||
private final LazilyConfiguredMapping<JavaLanguageVersion, AtomicReference<JavaInstallationMetadata>> jdks = | ||
new LazilyConfiguredMapping<>(AtomicReference::new); | ||
|
||
@Inject | ||
public BaselineJavaVersionsExtension(Project project) { | ||
|
@@ -45,9 +48,6 @@ public BaselineJavaVersionsExtension(Project project) { | |
libraryTarget.finalizeValueOnRead(); | ||
distributionTarget.finalizeValueOnRead(); | ||
runtime.finalizeValueOnRead(); | ||
|
||
jdks = project.getObjects().mapProperty(JavaLanguageVersion.class, JavaInstallationMetadata.class); | ||
jdks.finalizeValueOnRead(); | ||
} | ||
|
||
/** Target {@link JavaLanguageVersion} for compilation of libraries that are published. */ | ||
|
@@ -80,7 +80,20 @@ public final void setRuntime(int value) { | |
runtime.set(JavaLanguageVersion.of(value)); | ||
} | ||
|
||
public final MapProperty<JavaLanguageVersion, JavaInstallationMetadata> getJdks() { | ||
return jdks; | ||
public final Optional<JavaInstallationMetadata> jdkMetadataFor(JavaLanguageVersion javaLanguageVersion) { | ||
return jdks.get(javaLanguageVersion).map(AtomicReference::get); | ||
} | ||
|
||
public final void jdk(JavaLanguageVersion javaLanguageVersion, JavaInstallationMetadata javaInstallationMetadata) { | ||
jdks.put(javaLanguageVersion, ref -> ref.set(javaInstallationMetadata)); | ||
} | ||
|
||
public final void jdks(LazyJdks lazyJdks) { | ||
jdks.put(javaLanguageVersion -> lazyJdks.jdkFor(javaLanguageVersion) | ||
.map(javaInstallationMetadata -> ref -> ref.set(javaInstallationMetadata))); | ||
} | ||
|
||
public interface LazyJdks { | ||
Optional<JavaInstallationMetadata> jdkFor(JavaLanguageVersion javaLanguageVersion); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is basically making it fully lazy. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
/* | ||
* (c) Copyright 2022 Palantir Technologies Inc. All rights reserved. | ||
* | ||
* 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.palantir.baseline.plugins.javaversions; | ||
|
||
import java.util.ArrayList; | ||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
import java.util.concurrent.atomic.AtomicBoolean; | ||
import java.util.function.Supplier; | ||
import org.gradle.api.Action; | ||
|
||
public final class LazilyConfiguredMapping<K, V> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would hope to reuse this in gradle-jdks to have everything be ETE lazy. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. perhaps move to the extensions package and make package private? |
||
private final Supplier<V> valueFactory; | ||
private final List<LazyValues<K, V>> values = new ArrayList<>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might have been easier to set this as just a single function you have to replace - but I'm concerned that just leads to everything becoming confusing if you "override" the value in custom gradle for example to test a new JDK major version that hasn't been fully released yet. |
||
private final Map<K, Optional<V>> computedValues = new HashMap<>(); | ||
private boolean finalized = false; | ||
|
||
public LazilyConfiguredMapping(Supplier<V> valueFactory) { | ||
this.valueFactory = valueFactory; | ||
} | ||
|
||
public void put(LazyValues<K, V> lazyValues) { | ||
ensureNotFinalized(); | ||
|
||
values.add(lazyValues); | ||
} | ||
|
||
public void put(K key, Action<V> value) { | ||
ensureNotFinalized(); | ||
|
||
put(requestedKey -> { | ||
if (requestedKey.equals(key)) { | ||
return Optional.of(value); | ||
} | ||
|
||
return Optional.empty(); | ||
}); | ||
} | ||
|
||
private void ensureNotFinalized() { | ||
if (finalized) { | ||
throw new IllegalStateException(String.format( | ||
"This %s has already been finalized as get() hase been called. " | ||
+ "No further elements can be added to it", | ||
LazilyConfiguredMapping.class.getSimpleName())); | ||
} | ||
} | ||
|
||
public Optional<V> get(K key) { | ||
finalized = true; | ||
|
||
return computedValues.computeIfAbsent(key, _ignored -> { | ||
V value = valueFactory.get(); | ||
AtomicBoolean created = new AtomicBoolean(false); | ||
values.forEach(lazyValues -> { | ||
lazyValues.compute(key).ifPresent(action -> { | ||
created.set(true); | ||
action.execute(value); | ||
}); | ||
}); | ||
|
||
if (created.get()) { | ||
return Optional.of(value); | ||
} | ||
|
||
return Optional.empty(); | ||
}); | ||
} | ||
|
||
public interface LazyValues<K, V> { | ||
Optional<Action<V>> compute(K key); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
/* | ||
* (c) Copyright 2022 Palantir Technologies Inc. All rights reserved. | ||
* | ||
* 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.palantir.baseline.plugins.javaversions; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.assertj.core.api.Assertions.assertThatExceptionOfType; | ||
|
||
import java.util.Optional; | ||
import org.junit.jupiter.api.Test; | ||
|
||
class LazilyConfiguredMappingTest { | ||
private final LazilyConfiguredMapping<String, Extension> lazilyConfiguredMapping = | ||
new LazilyConfiguredMapping<>(() -> new Extension(0)); | ||
|
||
@Test | ||
void empty_mapping_returns_optional_empty() { | ||
assertThat(lazilyConfiguredMapping.get("abc")).isEmpty(); | ||
} | ||
|
||
@Test | ||
void can_put_a_value_and_get_it_out_again() { | ||
lazilyConfiguredMapping.put("foo", extension -> extension.number = 4); | ||
|
||
assertThat(lazilyConfiguredMapping.get("foo")).hasValue(new Extension(4)); | ||
} | ||
|
||
@Test | ||
void can_put_a_lazy_value_in_and_get_it_out_again() { | ||
lazilyConfiguredMapping.put(key -> Optional.of(extension -> extension.number = Integer.parseInt(key))); | ||
|
||
assertThat(lazilyConfiguredMapping.get("3")).hasValue(new Extension(3)); | ||
assertThat(lazilyConfiguredMapping.get("9")).hasValue(new Extension(9)); | ||
} | ||
|
||
@Test | ||
void lazy_values_are_able_to_not_return_values() { | ||
lazilyConfiguredMapping.put(_key -> Optional.empty()); | ||
|
||
assertThat(lazilyConfiguredMapping.get("abc")).isEmpty(); | ||
} | ||
|
||
@Test | ||
void interspersing_putting_values_takes_the_last_set_value() { | ||
lazilyConfiguredMapping.put("1", extension -> extension.number = 80); | ||
lazilyConfiguredMapping.put(key -> Optional.of(extension -> extension.number = Integer.parseInt(key))); | ||
lazilyConfiguredMapping.put("4", extension -> extension.number = 99); | ||
|
||
assertThat(lazilyConfiguredMapping.get("1")).hasValue(new Extension(1)); | ||
assertThat(lazilyConfiguredMapping.get("3")).hasValue(new Extension(3)); | ||
assertThat(lazilyConfiguredMapping.get("4")).hasValue(new Extension(99)); | ||
} | ||
|
||
@Test | ||
void throws_if_putting_values_after_being_finalized() { | ||
lazilyConfiguredMapping.get("abc"); | ||
|
||
assertThatExceptionOfType(IllegalStateException.class).isThrownBy(() -> { | ||
lazilyConfiguredMapping.put("foo", extension -> {}); | ||
}); | ||
|
||
assertThatExceptionOfType(IllegalStateException.class).isThrownBy(() -> { | ||
lazilyConfiguredMapping.put(key -> Optional.of(extension -> {})); | ||
}); | ||
} | ||
|
||
private static final class Extension { | ||
public int number; | ||
|
||
Extension(int number) { | ||
this.number = number; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object obj) { | ||
return number == ((Extension) obj).number; | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return number; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return Integer.toString(number); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use an
AtomicReference
here? I wanted to make a general type I could reuse in gradle-jdks to configure the versions/distributions, which would involve modifying extension objects in the regular gradle style. So we use anAtomicReference
here just to make it act like a box you can store data in.