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

Basic package manager implementation #448

Merged
merged 11 commits into from Aug 9, 2019
1 change: 1 addition & 0 deletions build.gradle
Expand Up @@ -101,6 +101,7 @@ dependencies {
compile group: 'org.terasology', name: 'CrashReporter', version: '1.2.+'
compile group: 'com.github.rjeschke', name: 'txtmark', version: '0.11'

compile 'com.google.code.gson:gson:2.8.5'
Copy link
Member

Choose a reason for hiding this comment

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

Please follow the same pattern as all the other dependencies.

Suggested change
compile 'com.google.code.gson:gson:2.8.5'
compile group: 'com.google.code.gson', name: 'gson', version: '2.8.5'

compile group: 'ch.qos.logback', name: 'logback-classic', version: '1.1.2'

// These dependencies are only needed for running tests
Expand Down
24 changes: 24 additions & 0 deletions src/main/java/org/terasology/launcher/packages/GamePackage.java
@@ -0,0 +1,24 @@
/*
* Copyright 2019 MovingBlocks
*
* 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 org.terasology.launcher.packages;

/**
* Model of a game package.
*/
public class GamePackage {
Copy link
Member

Choose a reason for hiding this comment

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

Class or interface? I guess easy to change later.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, it has to end up in some POJO form. Will be decided soon 👍

Copy link
Member

Choose a reason for hiding this comment

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

Some types have the Game prefix (like GamePackage), but others don't (PackageManager). Let's choose to either keep the prefix everywhere or get rid of it entirely :) Personally I'd prefer dropping the prefix to increase conciseness, but we can choose to retain it if it helps clarify the distinction between something being associated with the "game" or not. In other words, in my opinion the prefix makes sense only if a clear difference can be drawn between say Package and GamePackage.

Copy link
Member Author

Choose a reason for hiding this comment

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

I used GamePackage class as a placeholder for the actual model of a game package. And yes, I agree with your idea of dropping the 'game' from in front of it, since there are future plans to test it for modules too.

// TODO: Implement this
}
@@ -0,0 +1,39 @@
/*
* Copyright 2019 MovingBlocks
*
* 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 org.terasology.launcher.packages;

/**
* Enum for types of game package available now.
*/
public enum GamePackageType {
Copy link
Member

Choose a reason for hiding this comment

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

I'd consider adding a Build qualifier to the enum name, since it (currently) specifies which build the package is coming from.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll agree with that refactoring. There's actually a need for more concise plans on the package manager architecture, which also allows it to support multiple repository formats (Jenkins, GitHub pkg registry, other structured filesystems, etc.), where this enum won't be playing a central role probably.

STABLE("TerasologyStable"),
UNSTABLE("Terasology"),
OMEGA_STABLE("DistroOmegaRelease"),
OMEGA_UNSTABLE("DistroOmega");

// TODO: Remove this enum when 3rd-party package types are supported
Copy link
Member

Choose a reason for hiding this comment

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

Is the intent for the whole enum to go away in favor of JSON-based config files of some sort? I might suggest trying to work that out with even current state only supporting Jenkins.

Something like a module.txt but for packages, with description, url, provider type, etc. Might be something like that one piece to follow, even if there's no implementation (a design/convention from a different package manager that could be followed?)


private final String jobName;

GamePackageType(String jobName) {
this.jobName = jobName;
}

public String getJobName() {
return jobName;
}
}
82 changes: 82 additions & 0 deletions src/main/java/org/terasology/launcher/packages/JenkinsStorage.java
@@ -0,0 +1,82 @@
/*
* Copyright 2019 MovingBlocks
*
* 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 org.terasology.launcher.packages;

import com.google.gson.Gson;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.terasology.launcher.util.JobResult;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.net.URL;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

/**
* Provides game packages from our Jenkins server.
*/
public class JenkinsStorage implements Storage {
Copy link
Member

@eviltak eviltak Jul 25, 2019

Choose a reason for hiding this comment

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

In line with my comment on Storage above, I'd rename this to JenkinsRepository or similar.


private static final Logger logger = LoggerFactory.getLogger(JenkinsStorage.class);

private static final String JENKINS_JOB_URL = "http://jenkins.terasology.org/job/";
Copy link
Member

Choose a reason for hiding this comment

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

Much like the game package definition I would load these things from config files as well, not code (might already be intended for a future version?)

private static final String API_PATH = "/api/json?tree=builds[number,result]";
private static final int LIMIT_VERSIONS = 20;

private Gson gson = new Gson();

@Override
public List<Integer> getPackageVersions(GamePackageType pkgType) {
final String jobApiPath = JENKINS_JOB_URL + pkgType.getJobName() + API_PATH;

try (BufferedReader reader = new BufferedReader(
new InputStreamReader(
new URL(jobApiPath).openStream()))) {

final Job result = gson.fromJson(reader, Job.class);
return Arrays.stream(result.builds)
.filter(build -> build.result.equals(JobResult.SUCCESS.name()))
.map(build -> build.number)
.limit(LIMIT_VERSIONS)
.collect(Collectors.toList());

} catch (IOException e) {
logger.warn("Failed to access URL: {}", jobApiPath);
}
return Collections.emptyList();
}

@Override
public Optional<GamePackage> getPackage(GamePackageType pkgType, int version) {
// TODO: Implement this
return Optional.empty();
}

private static class Job {
private Build[] builds;
}

private static class Build {
private int number;
private String result;
}
}
104 changes: 104 additions & 0 deletions src/main/java/org/terasology/launcher/packages/LocalStorage.java
@@ -0,0 +1,104 @@
/*
* Copyright 2019 MovingBlocks
*
* 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 org.terasology.launcher.packages;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Collections;
import java.util.EnumMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;

/**
* Provides game packages from the local filesystem.
*/
public class LocalStorage implements Storage {
Copy link
Member

Choose a reason for hiding this comment

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

See main review comment.


private static final Logger logger = LoggerFactory.getLogger(LocalStorage.class);

private static final String VERSIONS_CACHE_FILENAME = "versions.cache";

private final Path versionsCache;
private final Map<GamePackageType, List<Integer>> cachedVersions;

LocalStorage(Path localDir) {
Copy link
Member

Choose a reason for hiding this comment

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

No access modifier? Intentional or nah? :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

IDE suggestions 😅

versionsCache = localDir.resolve(VERSIONS_CACHE_FILENAME);

final Map<GamePackageType, List<Integer>> cache = loadCache();
if (!cache.isEmpty()) {
cachedVersions = cache;
} else {
cachedVersions = new EnumMap<>(GamePackageType.class);
for (GamePackageType pkgType : GamePackageType.values()) {
cachedVersions.put(pkgType, Collections.emptyList());
}
}
}

void updateCache(GamePackageType pkgType, List<Integer> versions) {
Copy link
Member

Choose a reason for hiding this comment

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

More access modifier. The IDE might sometime offer premature advice about what access level is needed, but it is rare to use anything beyond public or private (let alone package access level on purpose)

cachedVersions.put(pkgType, versions);
logger.trace("Updating cached version list for {} ...", pkgType);
saveCache();
}

private void saveCache() {
try (ObjectOutputStream out = new ObjectOutputStream(
Files.newOutputStream(versionsCache))) {
out.writeObject(cachedVersions);
} catch (IOException e) {
logger.warn("Failed to write cache file: {}", versionsCache.toAbsolutePath());
}
}

private Map<GamePackageType, List<Integer>> loadCache() {
// TODO: Do this in a background thread
try (ObjectInputStream in = new ObjectInputStream(
Files.newInputStream(versionsCache))) {
return (Map<GamePackageType, List<Integer>>) in.readObject();
} catch (ClassNotFoundException | IOException e) {
logger.warn("Failed to load cache file: {}", versionsCache.toAbsolutePath());
}
return Collections.emptyMap();
}

void install(GamePackage pkg) {
// TODO: Implement this
}

void remove(GamePackage pkg) {
// TODO: Implement this
}

@Override
public List<Integer> getPackageVersions(GamePackageType pkgType) {
// TODO: Implement this
return Collections.emptyList();
}

@Override
public Optional<GamePackage> getPackage(GamePackageType pkgType, int version) {
// TODO: Implement this
return Optional.empty();
}
}
47 changes: 47 additions & 0 deletions src/main/java/org/terasology/launcher/packages/PackageManager.java
@@ -0,0 +1,47 @@
/*
* Copyright 2019 MovingBlocks
*
* 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 org.terasology.launcher.packages;

import java.nio.file.Path;

/**
* Handles installation, removal and update of game packages.
Copy link
Member

Choose a reason for hiding this comment

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

If this is the central-most class for the concept you might want to beef up the javadoc a fair amount 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

It'll be fully documented by the end 👍

*/
public class PackageManager {
private final Storage onlineStorage;
private final LocalStorage localStorage;

public PackageManager(Path localDir) {
Copy link
Member

Choose a reason for hiding this comment

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

A point of confusion for me: Are you using LocalStorage as another option for where the launcher can get packages from (so essentially a "local provide") or as a target for packages to be stored locally? Where's localDir intended to come from?

Copy link
Member Author

Choose a reason for hiding this comment

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

For now, it is designed only to receive packages (so not a 'Provider') and to install/remove them from the filesystem. 🙂 The localDir path (that points to local data dir) already gets loaded by the LauncherInitTask, and the package manager will just borrow it from there

onlineStorage = new JenkinsStorage();
localStorage = new LocalStorage(localDir);
}

public void sync() {
Copy link
Member

Choose a reason for hiding this comment

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

Javadoc again is super useful all over especially in placeholder code :-)

for (GamePackageType pkgType : GamePackageType.values()) {
localStorage.updateCache(pkgType, onlineStorage.getPackageVersions(pkgType));
}
}

public void install(GamePackageType pkgType, int version) {
// TODO: Install via cache
}

public void remove(GamePackageType pkgType, int version) {
localStorage.getPackage(pkgType, version)
.ifPresent(localStorage::remove);
}
}
28 changes: 28 additions & 0 deletions src/main/java/org/terasology/launcher/packages/Storage.java
@@ -0,0 +1,28 @@
/*
* Copyright 2019 MovingBlocks
*
* 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 org.terasology.launcher.packages;

import java.util.List;
import java.util.Optional;

/**
* Interface for anything that can store game packages.
*/
public interface Storage {
Copy link
Member

@eviltak eviltak Jul 25, 2019

Choose a reason for hiding this comment

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

I'd rename this to PackageRepository or similar, since its actually more of a repository -- all you can do is retrieve package information from it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, Repository might be a better name for it... even it's behavior is kinda similar to the Repository classes in Spring. I just made up the word Storage somehow, but my intentions were similar 🙂

List<Integer> getPackageVersions(GamePackageType pkgType);
Optional<GamePackage> getPackage(GamePackageType pkgType, int version);
}