adamcin commented on code in PR #31:
URL: 
https://github.com/apache/sling-org-apache-sling-testing-osgi-mock/pull/31#discussion_r1357002996


##########
core/src/main/java/org/apache/sling/testing/mock/osgi/config/annotations/UpdateConfig.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.sling.testing.mock.osgi.config.annotations;
+
+import org.osgi.service.component.annotations.Component;
+
+import java.lang.annotation.Repeatable;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+/**
+ * Define this annotation on a test class or method to use the {@link 
org.osgi.service.cm.ConfigurationAdmin} service
+ * to update the persisted properties for the configuration whose pid matches 
the {@link #pid()} attribute.
+ * Updates should be applied top-down for each test context scope, from with 
the outermost (class-level) to the
+ * innermost (method-level).
+ */
+@Repeatable(UpdateConfigs.class)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface UpdateConfig {
+
+    /**
+     * Specify a configuration pid to update with values specified by {@link 
#property()}. The default value is
+     * {@link Component#NAME}, which is a special string ("$") that can be 
used to specify the name of the
+     * {@link #component()} class as a configuration PID.
+     *
+     * @return a configuration pid
+     */
+    String pid() default Component.NAME;

Review Comment:
   @stefanseifert The empty string disables config lookup from 
ConfigurationAdmin, so that only the `property` attribute is considered for 
populating the type. My intention here was for testing component constructors 
in isolation using a component property type constructed as closely as possible 
to how `bnd` would construct the component descriptor from `@Component`, 
according to the OSGi spec. 
   
   The idea is to provide a constructed value that can be defined to closely 
match the combination of the service component's `@interface Config` and its 
`@Component(property = {})` as defined in the source file, to be able to 
express situations like this:
   
   ```java
   @Component(property = "service.ranking:Integer=500")
   @Designate(ocd = MyServiceImpl.Config.class)
   class MyServiceImpl implements MyService {
   
       @ObjectClassDefinition()
       public @interface Config {
   
           @AttributeDefinition()
           int service_ranking() default -1;
   
       }
   }
   ```
   
   The `@ConfigType` to define for representing the `bnd`-accurate default 
state is just two copy-pastes:
   
   ```java
   @ConfigType(type = MyServiceImpl.Config.class, property = 
"service.ranking:Integer=500")
   ```
   
   The `service.ranking` property in `@Component(property = 
"service.ranking:Integer=500")` takes precedence over the 
`Config.service_ranking()` attribute default, but a `service.ranking` property 
set on a bound configuration takes precedence over both.
   
   Because `@ConfigType` focuses on the construction of a component property 
type for use in a constructor or activation method in the test itself, I 
figured the defaults should be set to exclude consideration of complicating 
elements, and that includes the OCD-designated relationship with the parent 
component class. 
   
   On the other hand, I could try a different approach for signifying when the 
config loading is disabled, by using a different sentinel class for the 
`component` attribute default, like `Void.class`. Then I could use 
Component.NAME as the default pid for both `@ConfigType` and `@UpdateConfig` 
and make Void.class the same default `component` attribute. Then they are both 
consistent, and at the same time, not likely to invite unexpected side effects 
with ConfigurationAdmin and `registerInjectActivateService` because of 
`Object.class` being a more likely lazy pid and yet totally valid as a 
component name in its own right. I'll also treat an empty PID string the same 
as `Component.NAME`, because why not.



-- 
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: dev-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to