This is an automated email from the ASF dual-hosted git repository.
yufei pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new eca6e70f When a grant already exists, return the appropriate response
(#243)
eca6e70f is described below
commit eca6e70f1a84065c1a647b4bc94ec0524b5d774c
Author: Eric Maynard <[email protected]>
AuthorDate: Thu Oct 3 16:13:58 2024 -0400
When a grant already exists, return the appropriate response (#243)
---
.../PolarisEclipseLinkMetaStoreSessionImpl.java | 9 ++++++
.../core/exceptions/AlreadyExistsException.java | 33 ++++++++++++++++++++
.../polaris/core/exceptions/PolarisException.java | 35 ++++++++++++++++++++++
.../apache/polaris/service/PolarisApplication.java | 5 ++++
.../{ => exception}/IcebergExceptionMapper.java | 2 +-
.../IcebergJerseyViolationExceptionMapper.java | 2 +-
.../IcebergJsonProcessingExceptionMapper.java | 2 +-
.../PolarisExceptionMapper.java} | 35 ++++++++++++++--------
8 files changed, 107 insertions(+), 16 deletions(-)
diff --git
a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java
b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java
index 75d4530d..071d7fbc 100644
---
a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java
+++
b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java
@@ -27,6 +27,7 @@ import jakarta.persistence.EntityManagerFactory;
import jakarta.persistence.EntityTransaction;
import jakarta.persistence.OptimisticLockException;
import jakarta.persistence.Persistence;
+import jakarta.persistence.PersistenceException;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
@@ -34,6 +35,7 @@ import java.net.URL;
import java.net.URLClassLoader;
import java.util.HashMap;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
@@ -58,6 +60,7 @@ import org.apache.polaris.core.entity.PolarisEntityId;
import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PolarisGrantRecord;
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
+import org.apache.polaris.core.exceptions.AlreadyExistsException;
import org.apache.polaris.core.persistence.PolarisMetaStoreManagerImpl;
import org.apache.polaris.core.persistence.PolarisMetaStoreSession;
import org.apache.polaris.core.persistence.RetryOnConcurrencyException;
@@ -261,6 +264,12 @@ public class PolarisEclipseLinkMetaStoreSessionImpl
implements PolarisMetaStoreS
} finally {
localSession.remove();
}
+ } catch (PersistenceException e) {
+ if (e.toString().toLowerCase(Locale.ROOT).contains("duplicate key")) {
+ throw new AlreadyExistsException("Duplicate key error when persisting
entity", e);
+ } else {
+ throw e;
+ }
}
}
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/exceptions/AlreadyExistsException.java
b/polaris-core/src/main/java/org/apache/polaris/core/exceptions/AlreadyExistsException.java
new file mode 100644
index 00000000..a941a549
--- /dev/null
+++
b/polaris-core/src/main/java/org/apache/polaris/core/exceptions/AlreadyExistsException.java
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.polaris.core.exceptions;
+
+/**
+ * A {@link PolarisException} implementation for when Polaris is unable to
create an entity that
+ * already exists.
+ */
+public class AlreadyExistsException extends PolarisException {
+ public AlreadyExistsException(String message) {
+ super(message);
+ }
+
+ public AlreadyExistsException(String message, Throwable cause) {
+ super(message, cause);
+ }
+}
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/exceptions/PolarisException.java
b/polaris-core/src/main/java/org/apache/polaris/core/exceptions/PolarisException.java
new file mode 100644
index 00000000..96ef9cf7
--- /dev/null
+++
b/polaris-core/src/main/java/org/apache/polaris/core/exceptions/PolarisException.java
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.polaris.core.exceptions;
+
+/**
+ * Base class for Polaris-specific runtime exceptions.
+ *
+ * <p>All custom exceptions in Polaris should extend this class to provide
specific error details.
+ */
+public abstract class PolarisException extends RuntimeException {
+
+ public PolarisException(String message) {
+ super(message);
+ }
+
+ public PolarisException(String message, Throwable cause) {
+ super(message, cause);
+ }
+}
diff --git
a/polaris-service/src/main/java/org/apache/polaris/service/PolarisApplication.java
b/polaris-service/src/main/java/org/apache/polaris/service/PolarisApplication.java
index e62e8712..c782c6a9 100644
---
a/polaris-service/src/main/java/org/apache/polaris/service/PolarisApplication.java
+++
b/polaris-service/src/main/java/org/apache/polaris/service/PolarisApplication.java
@@ -100,6 +100,10 @@ import
org.apache.polaris.service.context.CallContextCatalogFactory;
import org.apache.polaris.service.context.CallContextResolver;
import org.apache.polaris.service.context.PolarisCallContextCatalogFactory;
import org.apache.polaris.service.context.RealmContextResolver;
+import org.apache.polaris.service.exception.IcebergExceptionMapper;
+import
org.apache.polaris.service.exception.IcebergJerseyViolationExceptionMapper;
+import
org.apache.polaris.service.exception.IcebergJsonProcessingExceptionMapper;
+import org.apache.polaris.service.exception.PolarisExceptionMapper;
import
org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory;
import org.apache.polaris.service.ratelimiter.RateLimiterFilter;
import
org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl;
@@ -277,6 +281,7 @@ public class PolarisApplication extends
Application<PolarisApplicationConfig> {
}
environment.jersey().register(new IcebergRestOAuth2Api(oauth2Service));
environment.jersey().register(new IcebergExceptionMapper());
+ environment.jersey().register(new PolarisExceptionMapper());
PolarisServiceImpl polarisService = new
PolarisServiceImpl(entityManagerFactory, authorizer);
environment.jersey().register(new PolarisCatalogsApi(polarisService));
environment.jersey().register(new PolarisPrincipalsApi(polarisService));
diff --git
a/polaris-service/src/main/java/org/apache/polaris/service/IcebergExceptionMapper.java
b/polaris-service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
similarity index 99%
rename from
polaris-service/src/main/java/org/apache/polaris/service/IcebergExceptionMapper.java
rename to
polaris-service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
index b829a39c..565b1a70 100644
---
a/polaris-service/src/main/java/org/apache/polaris/service/IcebergExceptionMapper.java
+++
b/polaris-service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.polaris.service;
+package org.apache.polaris.service.exception;
import jakarta.ws.rs.WebApplicationException;
import jakarta.ws.rs.core.MediaType;
diff --git
a/polaris-service/src/main/java/org/apache/polaris/service/IcebergJerseyViolationExceptionMapper.java
b/polaris-service/src/main/java/org/apache/polaris/service/exception/IcebergJerseyViolationExceptionMapper.java
similarity index 97%
copy from
polaris-service/src/main/java/org/apache/polaris/service/IcebergJerseyViolationExceptionMapper.java
copy to
polaris-service/src/main/java/org/apache/polaris/service/exception/IcebergJerseyViolationExceptionMapper.java
index 2ffbdbbb..e26e89a2 100644
---
a/polaris-service/src/main/java/org/apache/polaris/service/IcebergJerseyViolationExceptionMapper.java
+++
b/polaris-service/src/main/java/org/apache/polaris/service/exception/IcebergJerseyViolationExceptionMapper.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.polaris.service;
+package org.apache.polaris.service.exception;
import io.dropwizard.jersey.validation.JerseyViolationException;
import jakarta.ws.rs.core.MediaType;
diff --git
a/polaris-service/src/main/java/org/apache/polaris/service/IcebergJsonProcessingExceptionMapper.java
b/polaris-service/src/main/java/org/apache/polaris/service/exception/IcebergJsonProcessingExceptionMapper.java
similarity index 98%
rename from
polaris-service/src/main/java/org/apache/polaris/service/IcebergJsonProcessingExceptionMapper.java
rename to
polaris-service/src/main/java/org/apache/polaris/service/exception/IcebergJsonProcessingExceptionMapper.java
index 72992008..929453ae 100644
---
a/polaris-service/src/main/java/org/apache/polaris/service/IcebergJsonProcessingExceptionMapper.java
+++
b/polaris-service/src/main/java/org/apache/polaris/service/exception/IcebergJsonProcessingExceptionMapper.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.polaris.service;
+package org.apache.polaris.service.exception;
import com.fasterxml.jackson.core.JsonGenerationException;
import com.fasterxml.jackson.core.JsonParseException;
diff --git
a/polaris-service/src/main/java/org/apache/polaris/service/IcebergJerseyViolationExceptionMapper.java
b/polaris-service/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
similarity index 57%
rename from
polaris-service/src/main/java/org/apache/polaris/service/IcebergJerseyViolationExceptionMapper.java
rename to
polaris-service/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
index 2ffbdbbb..489b3735 100644
---
a/polaris-service/src/main/java/org/apache/polaris/service/IcebergJerseyViolationExceptionMapper.java
+++
b/polaris-service/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
@@ -16,34 +16,43 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.polaris.service;
+package org.apache.polaris.service.exception;
-import io.dropwizard.jersey.validation.JerseyViolationException;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.ext.ExceptionMapper;
import jakarta.ws.rs.ext.Provider;
import org.apache.iceberg.rest.responses.ErrorResponse;
+import org.apache.polaris.core.exceptions.AlreadyExistsException;
+import org.apache.polaris.core.exceptions.PolarisException;
/**
- * Override of the default JerseyViolationExceptionMapper to provide an
Iceberg ErrorResponse with
- * the exception details.
+ * An {@link ExceptionMapper} implementation for {@link PolarisException}s
modeled after {@link
+ * IcebergExceptionMapper}
*/
@Provider
-public class IcebergJerseyViolationExceptionMapper
- implements ExceptionMapper<JerseyViolationException> {
+public class PolarisExceptionMapper implements
ExceptionMapper<PolarisException> {
+
+ private Response.Status getStatus(PolarisException exception) {
+ if (exception instanceof AlreadyExistsException) {
+ return Response.Status.CONFLICT;
+ } else {
+ return Response.Status.INTERNAL_SERVER_ERROR;
+ }
+ }
+
@Override
- public Response toResponse(JerseyViolationException exception) {
- final String message = "Invalid value: " + exception.getMessage();
- ErrorResponse icebergErrorResponse =
+ public Response toResponse(PolarisException exception) {
+ Response.Status status = getStatus(exception);
+ ErrorResponse errorResponse =
ErrorResponse.builder()
- .responseCode(Response.Status.BAD_REQUEST.getStatusCode())
+ .responseCode(status.getStatusCode())
.withType(exception.getClass().getSimpleName())
- .withMessage(message)
+ .withMessage(exception.getMessage())
.build();
- return Response.status(Response.Status.BAD_REQUEST)
+ return Response.status(status)
+ .entity(errorResponse)
.type(MediaType.APPLICATION_JSON_TYPE)
- .entity(icebergErrorResponse)
.build();
}
}
