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

Add more retryable exceptions + cleanup #491

Merged
merged 3 commits into from
Dec 19, 2015
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 @@ -25,7 +25,7 @@
import java.util.Objects;

/**
* Base class for Resource Manager operation options
* Base class for Resource Manager operation options.
*/
class Option implements Serializable {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
/**
* A Google Cloud Resource Manager project object.
*
* A Project is a high-level Google Cloud Platform entity. It is a container for ACLs, APIs,
* <p>A Project is a high-level Google Cloud Platform entity. It is a container for ACLs, APIs,
* AppEngine Apps, VMs, and other Google Cloud Platform resources. This class' member variables are
* immutable. Methods that change or update the underlying Project information return a new Project
* instance.
Expand Down Expand Up @@ -77,7 +77,7 @@ public Project reload() {
/**
* Marks the project identified by the specified project ID for deletion.
*
* This method will only affect the project if the following criteria are met:
* <p>This method will only affect the project if the following criteria are met:
* <ul>
* <li>The project does not have a billing account associated with it.
* <li>The project has a lifecycle state of {@link ProjectInfo.State#ACTIVE}.
Expand All @@ -103,7 +103,7 @@ public void delete() {
/**
* Restores the project identified by the specified project ID.
*
* You can only use this method for a project that has a lifecycle state of
* <p>You can only use this method for a project that has a lifecycle state of
* {@link ProjectInfo.State#DELETE_REQUESTED}. After deletion starts, as indicated by a lifecycle
* state of {@link ProjectInfo.State#DELETE_IN_PROGRESS}, the project cannot be restored. The
* caller must have modify permissions for this project.
Expand All @@ -120,7 +120,7 @@ public void undelete() {
/**
* Replaces the attributes of the project.
*
* The caller must have modify permissions for this project.
* <p>The caller must have modify permissions for this project.
*
* @see <a
* href="https://cloud.google.com/resource-manager/reference/rest/v1beta1/projects/update">Cloud
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import com.google.gcloud.Service;
import com.google.gcloud.spi.ResourceManagerRpc;

import java.util.HashSet;
import java.util.Set;

/**
* An interface for Google Cloud Resource Manager.
Expand All @@ -31,7 +31,7 @@
*/
public interface ResourceManager extends Service<ResourceManagerOptions> {

public static final String DEFAULT_CONTENT_TYPE = "application/octet-stream";
String DEFAULT_CONTENT_TYPE = "application/octet-stream";

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.


/**
* The fields of a project.
Expand Down Expand Up @@ -59,7 +59,7 @@ public String selector() {
}

static String selector(ProjectField... fields) {
HashSet<String> fieldStrings = Sets.newHashSetWithExpectedSize(fields.length + 1);
Set<String> fieldStrings = Sets.newHashSetWithExpectedSize(fields.length + 1);
fieldStrings.add(PROJECT_ID.selector());
for (ProjectField field : fields) {
fieldStrings.add(field.selector());
Expand All @@ -71,7 +71,7 @@ static String selector(ProjectField... fields) {
/**
* Class for specifying project get options.
*/
public class ProjectGetOption extends Option {
class ProjectGetOption extends Option {

private static final long serialVersionUID = 270185129961146874L;

Expand All @@ -95,7 +95,7 @@ public static ProjectGetOption fields(ProjectField... fields) {
/**
* Class for specifying project list options.
*/
public class ProjectListOption extends Option {
class ProjectListOption extends Option {

private static final long serialVersionUID = 7888768979702012328L;

Expand Down Expand Up @@ -213,8 +213,8 @@ public static ProjectListOption fields(ProjectField... fields) {
/**
* Retrieves the project identified by the specified project ID.
*
* <p>The caller must have read permissions for this project. Returns {@code null} if project
* not found.
* <p>Returns {@code null} if the project is not found or if the user doesn't have read
* permissions for the project.
*
* @see <a
* href="https://cloud.google.com/resource-manager/reference/rest/v1beta1/projects/get">Cloud
Expand All @@ -227,10 +227,9 @@ public static ProjectListOption fields(ProjectField... fields) {
* Lists the projects visible to the current user.
*
* <p>This method returns projects in an unspecified order. New projects do not necessarily appear
* at
* the end of the list. Use {@link ProjectListOption} to filter this list, set page size, and set
* page tokens. Note that pagination is currently not implemented by the Cloud Resource Manager
* API.
* at the end of the list. Use {@link ProjectListOption} to filter this list, set page size, and
* set page tokens. Note that pagination is currently not implemented by the Cloud Resource
* Manager API.
*
* @see <a
* href="https://cloud.google.com/resource-manager/reference/rest/v1beta1/projects/list">Cloud
Expand Down Expand Up @@ -264,7 +263,7 @@ public static ProjectListOption fields(ProjectField... fields) {
* @see <a
* href="https://cloud.google.com/resource-manager/reference/rest/v1beta1/projects/undelete">Cloud
* Resource Manager undelete</a>
* @throws ResourceManagerException
* @throws ResourceManagerException upon failure
*/
void undelete(String projectId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ public boolean equals(Object obj) {
return obj instanceof ResourceManagerOptions && baseEquals((ResourceManagerOptions) obj);
}

@Override
public int hashCode() {
return baseHashCode();
}

@Override
public Builder toBuilder() {
return new Builder(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public class LocalResourceManagerHelper {
private static final Set<Character> PERMISSIBLE_PROJECT_NAME_PUNCTUATION =
ImmutableSet.of('-', '\'', '"', ' ', '!');

private HttpServer server;
private final HttpServer server;
private final ConcurrentHashMap<String, Project> projects = new ConcurrentHashMap<>();
private final int port;

Expand Down Expand Up @@ -257,7 +257,7 @@ private static Map<String, Object> parseListOptions(String query) {
return options;
}

private static final String checkForProjectErrors(Project project) {
private static String checkForProjectErrors(Project project) {
if (project.getProjectId() == null) {
return "Project ID cannot be empty.";
}
Expand Down Expand Up @@ -291,9 +291,9 @@ private static final String checkForProjectErrors(Project project) {
return null;
}

private static final boolean isValidIdOrLabel(String value, int minLength, int maxLength) {
private static boolean isValidIdOrLabel(String value, int minLength, int maxLength) {
for (char c : value.toCharArray()) {
if (c != '-' && !Character.isDigit(c) && (!Character.isLowerCase(c))) {
if (c != '-' && !Character.isDigit(c) && !Character.isLowerCase(c)) {
return false;
}
}
Expand Down Expand Up @@ -367,7 +367,7 @@ Response list(Map<String, Object> options) {
}
String[] fields = (String[]) options.get("fields");
for (Project p : projects.values()) {
Boolean includeProject = includeProject(p, filters);
boolean includeProject = includeProject(p, filters);
if (includeProject) {
try {
projectsSerialized.add(jsonFactory.toString(extractFields(p, fields)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* A testing helper for Google Cloud Resource Manager.
*
* <p>A simple usage example:
* <p>Before the test:
* Before the test:
* <pre> {@code
* LocalResourceManagerHelper resourceManagerHelper = LocalResourceManagerHelper.create();
* ResourceManager resourceManager = resourceManagerHelper.options().service();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import static com.google.gcloud.spi.ResourceManagerRpc.Option.PAGE_SIZE;
import static com.google.gcloud.spi.ResourceManagerRpc.Option.PAGE_TOKEN;
import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
import static java.net.HttpURLConnection.HTTP_NOT_FOUND;

import com.google.api.client.googleapis.json.GoogleJsonError;
import com.google.api.client.googleapis.json.GoogleJsonResponseException;
Expand All @@ -26,6 +27,9 @@ public class DefaultResourceManagerRpc implements ResourceManagerRpc {

// see https://cloud.google.com/resource-manager/v1/errors/core_errors
private static final Set<Integer> RETRYABLE_CODES = ImmutableSet.of(503, 500, 429);
private static final Set<String> RETRYABLE_REASONS = ImmutableSet.of("concurrentLimitExceeded",
"limitExceeded", "rateLimitExceeded", "rateLimitExceededUnreg", "servingLimitExceeded",
"userRateLimitExceeded", "userRateLimitExceededUnreg", "variableTermLimitExceeded");

private final Cloudresourcemanager resourceManager;

Expand All @@ -51,7 +55,9 @@ private static ResourceManagerException translate(IOException exception) {
}

private static ResourceManagerException translate(GoogleJsonError exception) {
boolean retryable = RETRYABLE_CODES.contains(exception.getCode());
boolean retryable =
RETRYABLE_CODES.contains(exception.getCode()) || (!exception.getErrors().isEmpty()
&& RETRYABLE_REASONS.contains(exception.getErrors().get(0).getReason()));
return new ResourceManagerException(exception.getCode(), exception.getMessage(), retryable);
}

Expand Down Expand Up @@ -82,8 +88,9 @@ public Project get(String projectId, Map<Option, ?> options) throws ResourceMana
.execute();
} catch (IOException ex) {
ResourceManagerException translated = translate(ex);
if (translated.code() == HTTP_FORBIDDEN) {
return null; // Project not found
if (translated.code() == HTTP_FORBIDDEN || translated.code() == HTTP_NOT_FOUND) {

This comment was marked as spam.

This comment was marked as spam.

// Service can return either 403 or 404 to signify that the project doesn't exist.
return null;
} else {
throw translated;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ private static com.google.api.services.cloudresourcemanager.model.Project copyFr
.setProjectId(from.getProjectId())
.setName(from.getName())
.setLabels(from.getLabels() != null ? ImmutableMap.copyOf(from.getLabels()) : null)
.setProjectNumber(
from.getProjectNumber() != null ? from.getProjectNumber().longValue() : null)
.setProjectNumber(from.getProjectNumber())
.setCreateTime(from.getCreateTime())
.setLifecycleState(from.getLifecycleState())
.setParent(from.getParent() != null ? from.getParent().clone() : null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ public void testListFilterOptions() {
@Test
public void testReplace() {
ProjectInfo createdProject = RESOURCE_MANAGER.create(COMPLETE_PROJECT);
String newName = "new name";
Map<String, String> newLabels = ImmutableMap.of("new k1", "new v1");
ProjectInfo anotherCompleteProject = ProjectInfo.builder(COMPLETE_PROJECT.projectId())
.labels(newLabels)
Expand Down