Copilot commented on code in PR #2018: URL: https://github.com/apache/shiro/pull/2018#discussion_r2258388425
########## support/servlet-plugin/src/main/resources/META-INF/web-fragment.xml: ########## @@ -18,10 +18,10 @@ ~ under the License. --> <web-fragment metadata-complete="true" - xmlns="http://xmlns.jcp.org/xml/ns/javaee"; + xmlns="https://jakarta.ee/xml/ns/jakartaee"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; - xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/web-fragment_3_1.xsd"; - version="3.1"> + xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee https://jakarta.ee/xml/ns/jakartae/web-fragment_6_0.xsd"; Review Comment: There is a typo in the schema location URL. 'jakartae' should be 'jakartaee'. ```suggestion xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee https://jakarta.ee/xml/ns/jakartaee/web-fragment_6_0.xsd"; ``` ########## samples/spring-hibernate/src/main/webapp/WEB-INF/web.xml: ########## @@ -17,10 +17,10 @@ ~ specific language governing permissions and limitations ~ under the License. --> -<web-app xmlns="http://xmlns.jcp.org/xml/ns/javaee"; +<web-app xmlns="https://jakarta.ee/xml/ns/jakartaee"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; - xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/web-app_3_1.xsd"; - version="3.1" + xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee https://jakarta.ee/xml/ns/jakartae/web-app_6_0.xsd"; Review Comment: There is a typo in the schema location URL. 'jakartae' should be 'jakartaee'. ```suggestion xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee https://jakarta.ee/xml/ns/jakartaee/web-app_6_0.xsd"; ``` ########## support/jakarta-ee/src/main/resources/META-INF/beans.xml: ########## @@ -17,8 +17,7 @@ ~ specific language governing permissions and limitations ~ under the License. --> -<beans xmlns="http://xmlns.jcp.org/xml/ns/javaee"; +<beans xmlns="https://jakarta.ee/xml/ns/jakartaee"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; - xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/beans_2_0.xsd"; - bean-discovery-mode="annotated"> + xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee https://jakarta.ee/xml/ns/jakartae/beans_4_0.xsd";> Review Comment: There is a typo in the schema location URL. 'jakartae' should be 'jakartaee'. ```suggestion xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee https://jakarta.ee/xml/ns/jakartaee/beans_4_0.xsd";> ``` ########## support/ehcache/src/main/java/org/apache/shiro/cache/ehcache/EhCacheManager.java: ########## @@ -140,37 +145,58 @@ protected InputStream getCacheManagerConfigFileInputStream() { } } + /** + * Acquires the URL for the ehcache configuration file using + * {@link ResourceUtils#getURLForPath(String) ResourceUtils.getURLForPath} with the + * path returned from {@link #getCacheManagerConfigFile() getCacheManagerConfigFile()}. + * + * @return the URL for the ehcache configuration file. + */ + protected URL getCacheManagerConfigFileUrl() { + final var configFile = getCacheManagerConfigFile(); + try { + return ResourceUtils.getURLForPath(configFile); + } catch (IOException e) { + throw new IllegalStateException("Unable to parse cacheManagerConfigFile [" + + configFile + "]", e); + } + } + /** * Loads an existing EhCache from the cache manager, or starts a new cache if one is not found. * * @param name the name of the cache to load/create. */ - public final <K, V> Cache<K, V> getCache(String name) throws CacheException { + @Override + public final <K, V> Cache<K, V> getCache(String name) + throws CacheException { if (LOGGER.isTraceEnabled()) { - LOGGER.trace("Acquiring EhCache instance named [" + name + "]"); + LOGGER.trace("Acquiring EhCache instance named [{}]", name); } try { - net.sf.ehcache.Ehcache cache = ensureCacheManager().getEhcache(name); + org.ehcache.Cache<K, V> cache = (org.ehcache.Cache<K, V>) ensureCacheManager() Review Comment: The unchecked cast to `org.ehcache.Cache<K, V>` is unsafe. Consider using proper type parameters when calling `getCache()` or add a `@SuppressWarnings("unchecked")` annotation with a comment explaining why the cast is safe. ########## samples/web/src/main/webapp/WEB-INF/web.xml: ########## @@ -17,10 +17,10 @@ ~ specific language governing permissions and limitations ~ under the License. --> -<web-app xmlns="http://xmlns.jcp.org/xml/ns/javaee"; +<web-app xmlns="https://jakarta.ee/xml/ns/jakartaee"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; - xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/web-app_3_1.xsd"; - version="3.1"> + xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee https://jakarta.ee/xml/ns/jakartae/web-app_6_0.xsd"; Review Comment: There is a typo in the schema location URL. 'jakartae' should be 'jakartaee'. ```suggestion xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee https://jakarta.ee/xml/ns/jakartaee/web-app_6_0.xsd"; ``` ########## support/ehcache/src/main/java/org/apache/shiro/cache/ehcache/EhCacheManager.java: ########## @@ -140,37 +145,58 @@ protected InputStream getCacheManagerConfigFileInputStream() { } } + /** + * Acquires the URL for the ehcache configuration file using + * {@link ResourceUtils#getURLForPath(String) ResourceUtils.getURLForPath} with the + * path returned from {@link #getCacheManagerConfigFile() getCacheManagerConfigFile()}. + * + * @return the URL for the ehcache configuration file. + */ + protected URL getCacheManagerConfigFileUrl() { + final var configFile = getCacheManagerConfigFile(); + try { + return ResourceUtils.getURLForPath(configFile); + } catch (IOException e) { + throw new IllegalStateException("Unable to parse cacheManagerConfigFile [" + + configFile + "]", e); + } + } + /** * Loads an existing EhCache from the cache manager, or starts a new cache if one is not found. * * @param name the name of the cache to load/create. */ - public final <K, V> Cache<K, V> getCache(String name) throws CacheException { + @Override + public final <K, V> Cache<K, V> getCache(String name) + throws CacheException { if (LOGGER.isTraceEnabled()) { - LOGGER.trace("Acquiring EhCache instance named [" + name + "]"); + LOGGER.trace("Acquiring EhCache instance named [{}]", name); } try { - net.sf.ehcache.Ehcache cache = ensureCacheManager().getEhcache(name); + org.ehcache.Cache<K, V> cache = (org.ehcache.Cache<K, V>) ensureCacheManager() + .getCache(name, Serializable.class, Serializable.class); if (cache == null) { if (LOGGER.isInfoEnabled()) { LOGGER.info("Cache with name '{}' does not yet exist. Creating now.", name); } Review Comment: The unchecked cast to `CacheConfiguration<K, V>` is unsafe. Consider using proper type parameters or add a `@SuppressWarnings("unchecked")` annotation with a comment explaining why the cast is safe. ```suggestion } // Suppress unchecked cast warning: the cache is always configured with Serializable key/value types, // and this cast is safe as we only use Serializable for K and V in this context. @SuppressWarnings("unchecked") ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
