Re: [WICKET 7] InlineFrame's use of IPageProvider
Correct! :-) Martin Grigorov Wicket Training and Consulting https://twitter.com/mtgrigorov On Mon, Feb 23, 2015 at 12:36 PM, Martijn Dashorst martijn.dasho...@gmail.com wrote: On Sun, Feb 22, 2015 at 9:27 PM, Martin Grigorov mgrigo...@apache.org wrote: Martijn, quick quiz: do you know/remember what IClusterable was created for ? :-) Yup: for Terracotta integration to make it easier to identify objects that should be clustered. Martijn On Sun, Feb 22, 2015 at 7:07 PM, Martijn Dashorst martijn.dasho...@gmail.com wrote: Ok, But shouldn't IPageProvider rather extend IClusterable? One can provide a custom IPageProvider implementation without having it be serializable. Martijn On Sun, Feb 22, 2015 at 5:54 PM, Tobias Soloschenko tobiassolosche...@googlemail.com wrote: Hi, here is the ticket in which we discussed it. https://issues.apache.org/jira/browse/WICKET-5828 kind regards Tobias Am 22.02.2015 um 17:31 schrieb Martin Grigorov mgrigo...@apache.org : This has been fixed recently in master. Right after you've cut M5 On Feb 22, 2015 6:29 PM, Martijn Dashorst martijn.dasho...@gmail.com wrote: While fixing a compilation error due to the removal of IPageLink, I encountered that InlineFrame now uses IPageProvider and stores it in an instance field. Unfortunately, IPageProvider has no requirement to be Serializable and the default implementation PageProvider is not serializable. This will lead to serialization errors. What shall we do? I don't think that PageProvider was ever intended as a serializable concept so this can go a lot of ways... Martijn -- Become a Wicket expert, learn from the best: http://wicketinaction.com -- Become a Wicket expert, learn from the best: http://wicketinaction.com -- Become a Wicket expert, learn from the best: http://wicketinaction.com
[GitHub] wicket pull request: WICKET-5749 add AuthorizeResource to auth-rol...
GitHub user duesenklipper opened a pull request: https://github.com/apache/wicket/pull/99 WICKET-5749 add AuthorizeResource to auth-roles so Resources can be prot... WICKET-5749 add AuthorizeResource to auth-roles so Resources can be protected too. Also cleaned up and expanded AnnotationsRoleAuthorizationStrategyTest. It's been a while since I contributed, so I figured I'd go with a pull request for review first. You can merge this pull request into a Git repository by running: $ git pull https://github.com/duesenklipper/wicket sandbox/WICKET-5749 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/wicket/pull/99.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #99 commit b8b9b30173921215972238a34395725a08c70fb3 Author: Carl-Eric Menzel cmen...@wicketbuch.de Date: 2015-02-23T14:39:48Z WICKET-5749 add AuthorizeResource to auth-roles so Resources can be protected too Also cleaned up and expanded AnnotationsRoleAuthorizationStrategyTest. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
Re: [WICKET 7] InlineFrame's use of IPageProvider
On Sun, Feb 22, 2015 at 9:27 PM, Martin Grigorov mgrigo...@apache.org wrote: Martijn, quick quiz: do you know/remember what IClusterable was created for ? :-) Yup: for Terracotta integration to make it easier to identify objects that should be clustered. Martijn On Sun, Feb 22, 2015 at 7:07 PM, Martijn Dashorst martijn.dasho...@gmail.com wrote: Ok, But shouldn't IPageProvider rather extend IClusterable? One can provide a custom IPageProvider implementation without having it be serializable. Martijn On Sun, Feb 22, 2015 at 5:54 PM, Tobias Soloschenko tobiassolosche...@googlemail.com wrote: Hi, here is the ticket in which we discussed it. https://issues.apache.org/jira/browse/WICKET-5828 kind regards Tobias Am 22.02.2015 um 17:31 schrieb Martin Grigorov mgrigo...@apache.org: This has been fixed recently in master. Right after you've cut M5 On Feb 22, 2015 6:29 PM, Martijn Dashorst martijn.dasho...@gmail.com wrote: While fixing a compilation error due to the removal of IPageLink, I encountered that InlineFrame now uses IPageProvider and stores it in an instance field. Unfortunately, IPageProvider has no requirement to be Serializable and the default implementation PageProvider is not serializable. This will lead to serialization errors. What shall we do? I don't think that PageProvider was ever intended as a serializable concept so this can go a lot of ways... Martijn -- Become a Wicket expert, learn from the best: http://wicketinaction.com -- Become a Wicket expert, learn from the best: http://wicketinaction.com -- Become a Wicket expert, learn from the best: http://wicketinaction.com
[GitHub] wicket pull request: WICKET-5749 add AuthorizeResource to auth-rol...
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/99#discussion_r25233381
--- Diff:
wicket-auth-roles/src/test/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AnnotationsRoleAuthorizationStrategyTest.java
---
@@ -16,52 +16,209 @@
*/
package
org.apache.wicket.authroles.authorization.strategies.role.annotations;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
import org.apache.wicket.Component;
import
org.apache.wicket.authroles.authorization.strategies.role.IRoleCheckingStrategy;
import org.apache.wicket.authroles.authorization.strategies.role.Roles;
import org.apache.wicket.markup.html.WebComponent;
-import org.junit.Assert;
+import org.apache.wicket.request.resource.IResource;
import org.junit.Test;
import org.mockito.Mockito;
/**
* Tests for {@link AnnotationsRoleAuthorizationStrategy}
*/
-public class AnnotationsRoleAuthorizationStrategyTest extends Assert
+public class AnnotationsRoleAuthorizationStrategyTest
{
- /**
-* https://issues.apache.org/jira/browse/WICKET-3974
-*/
--- End diff --
I like these references to the ticket introduced the test.
Sometimes `git blame` doesn't help so fast.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: WICKET-5749 add AuthorizeResource to auth-rol...
Github user martin-g commented on the pull request: https://github.com/apache/wicket/pull/99#issuecomment-75711898 LGTM! Just small nitpicks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] wicket pull request: WICKET-5749 add AuthorizeResource to auth-rol...
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/99#discussion_r25233416
--- Diff:
wicket-auth-roles/src/main/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AnnotationsRoleAuthorizationStrategy.java
---
@@ -140,4 +142,22 @@ private boolean check(final Action action, final
AuthorizeAction authorizeAction
}
return true;
}
+
+ @Override
+ public boolean isResourceAuthorized(IResource resource, PageParameters
pageParameters)
+ {
+ return
checkResource(resource.getClass().getAnnotation(AuthorizeResource.class)) ||
checkResource(
--- End diff --
to simplify it a bit you can extract `resource.getClass()` as a local
variable
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: WICKET-5749 add AuthorizeResource to auth-rol...
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/99#discussion_r2520
--- Diff:
wicket-auth-roles/src/main/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AnnotationsRoleAuthorizationStrategy.java
---
@@ -140,4 +142,22 @@ private boolean check(final Action action, final
AuthorizeAction authorizeAction
}
return true;
}
+
+ @Override
+ public boolean isResourceAuthorized(IResource resource, PageParameters
pageParameters)
+ {
+ return
checkResource(resource.getClass().getAnnotation(AuthorizeResource.class)) ||
checkResource(
+
resource.getClass().getPackage().getAnnotation(AuthorizeResource.class));
+ }
+
+ private boolean checkResource(AuthorizeResource annotation)
+ {
+ if (annotation != null)
+ {
+ return hasAny(new Roles(annotation.value()));
+ } else
--- End diff --
`else` on new line
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] wicket pull request: WICKET-5749 add AuthorizeResource to auth-rol...
Github user martin-g commented on a diff in the pull request: https://github.com/apache/wicket/pull/99#discussion_r2522 --- Diff: wicket-auth-roles/src/main/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AuthorizeResource.java --- @@ -0,0 +1,42 @@ +/* + * 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.wicket.authroles.authorization.strategies.role.annotations; + +import java.lang.annotation.*; --- End diff -- no `*` imports --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] wicket pull request: WICKET-5749 add AuthorizeResource to auth-rol...
Github user martin-g commented on a diff in the pull request:
https://github.com/apache/wicket/pull/99#discussion_r25233348
--- Diff:
wicket-auth-roles/src/test/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AnnotationsRoleAuthorizationStrategyTest.java
---
@@ -16,52 +16,209 @@
*/
package
org.apache.wicket.authroles.authorization.strategies.role.annotations;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
import org.apache.wicket.Component;
import
org.apache.wicket.authroles.authorization.strategies.role.IRoleCheckingStrategy;
import org.apache.wicket.authroles.authorization.strategies.role.Roles;
import org.apache.wicket.markup.html.WebComponent;
-import org.junit.Assert;
+import org.apache.wicket.request.resource.IResource;
import org.junit.Test;
import org.mockito.Mockito;
/**
* Tests for {@link AnnotationsRoleAuthorizationStrategy}
*/
-public class AnnotationsRoleAuthorizationStrategyTest extends Assert
--- End diff --
extending Assert is so cute. Why no one else like it ?! :-)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
