Skip to content
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

Disable URLConnection cache in CLI #180

Merged
merged 1 commit into from
Oct 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public void execute(Arguments arguments, ClassLoader classLoader) {
}

private ValidatedResult<Model> buildModel(ClassLoader classLoader, List<String> models, Arguments arguments) {
ModelAssembler assembler = Model.assembler(classLoader);
ModelAssembler assembler = CommandUtils.createModelAssembler(classLoader);
CommandUtils.handleModelDiscovery(arguments, assembler, classLoader);
CommandUtils.handleUnknownTraitsOption(arguments, assembler);
models.forEach(assembler::addImport);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import software.amazon.smithy.cli.Arguments;
import software.amazon.smithy.cli.CliError;
import software.amazon.smithy.cli.SmithyCli;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.loader.ModelAssembler;

final class CommandUtils {
Expand All @@ -39,6 +40,10 @@ static void handleUnknownTraitsOption(Arguments arguments, ModelAssembler assemb
}
}

static ModelAssembler createModelAssembler(ClassLoader classLoader) {
return Model.assembler(classLoader).putProperty(ModelAssembler.DISABLE_JAR_CACHE, true);
}

static void handleModelDiscovery(Arguments arguments, ModelAssembler assembler, ClassLoader baseLoader) {
if (arguments.has(SmithyCli.DISCOVER_CLASSPATH)) {
discoverModelsWithClasspath(arguments, assembler);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public void execute(Arguments arguments, ClassLoader classLoader) {
List<String> newModels = arguments.repeatedParameter("--new");
LOGGER.info(String.format("Setting 'new' Smithy models: %s", String.join(" ", newModels)));

ModelAssembler assembler = Model.assembler(classLoader);
ModelAssembler assembler = CommandUtils.createModelAssembler(classLoader);
Model oldModel = loadModel("old", assembler, oldModels);
assembler.reset();
Model newModel = loadModel("new", assembler, newModels);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public void execute(Arguments arguments, ClassLoader classLoader) {
List<String> models = arguments.positionalArguments();
LOGGER.info(String.format("Validating Smithy model sources: %s", models));

ModelAssembler assembler = Model.assembler(classLoader);
ModelAssembler assembler = CommandUtils.createModelAssembler(classLoader);
CommandUtils.handleModelDiscovery(arguments, assembler, classLoader);
CommandUtils.handleUnknownTraitsOption(arguments, assembler);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.net.URLConnection;
import java.util.function.Supplier;
import java.util.logging.Logger;
import software.amazon.smithy.utils.IoUtils;
Expand Down Expand Up @@ -45,9 +46,17 @@ public boolean load(String filename, Supplier<String> contentSupplier, LoaderVis
LOGGER.fine(() -> "Loading Smithy model imports from JAR: " + manifestUrl);

for (URL model : ModelDiscovery.findModels(manifestUrl)) {
try (InputStream is = model.openStream()) {
String contents = IoUtils.toUtf8String(is);
delegate.load(model.toExternalForm(), () -> contents, visitor);
try {
URLConnection connection = model.openConnection();

if (visitor.hasProperty(ModelAssembler.DISABLE_JAR_CACHE)) {
connection.setUseCaches(false);
}

try (InputStream is = connection.getInputStream()) {
String contents = IoUtils.toUtf8String(is);
delegate.load(model.toExternalForm(), () -> contents, visitor);
}
} catch (IOException e) {
throw new ModelImportException(
String.format("Error loading Smithy model from URL `%s`: %s", model, e.getMessage()), e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.io.InputStream;
import java.io.UncheckedIOException;
import java.net.URL;
import java.net.URLConnection;
import java.nio.file.FileVisitOption;
import java.nio.file.Files;
import java.nio.file.Path;
Expand Down Expand Up @@ -61,7 +62,22 @@
* @see Model#assembler()
*/
public final class ModelAssembler {
/**
* Allow unknown traits rather than fail.
*/
public static final String ALLOW_UNKNOWN_TRAITS = "assembler.allowUnknownTraits";

/**
* Sets {@link URLConnection#setUseCaches} to false.
*
* <p>When running in a build environment, using caches can cause exceptions
* like `java.util.zip.ZipException: ZipFile invalid LOC header (bad signature)`
* because a previously loaded JAR might change between builds. The
* "assembler.disableJarCache" setting should be set to true when embedding
* Smithy into an environment where this can occur.
*/
public static final String DISABLE_JAR_CACHE = "assembler.disableJarCache";

private static final Logger LOGGER = Logger.getLogger(ModelAssembler.class.getName());
private static final ModelLoader DEFAULT_LOADER = ModelLoader.createDefaultLoader();

Expand Down Expand Up @@ -126,7 +142,6 @@ public ModelAssembler copy() {
* <li>Models registered via {@link #addModel}</li>
* <li>Shape registered via {@link #addModel}</li>
* <li>Metadata registered via {@link #putMetadata}</li>
* <li>Custom properties set via {@link #putProperty}</li>
* </ul>
*
* <p>The state of {@link #disablePrelude} is reset such that the prelude
Expand All @@ -142,7 +157,6 @@ public ModelAssembler reset() {
validators.clear();
suppressions.clear();
documentNodes.clear();
properties.clear();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These properties should have never of been cleared during a call to reset(). Reset should clear out transient state of the assembler, not configuration like properties.

disablePrelude = false;
return this;
}
Expand Down