Uploaded image for project: 'camunda BPM'
  1. camunda BPM
  2. CAM-7945

deploymentBuilder#addXXX(...) should only accept supported resource names

    Details

    • Type: Feature Request
    • Status: Closed
    • Priority: L3 - Default
    • Resolution: Duplicate
    • Affects Version/s: 7.7.0
    • Fix Version/s: None
    • Component/s: engine
    • Labels:
      None

      Description

      I had this effect myself when deploying a BpmnModelInstance and also found this on stackoverflow now (https://stackoverflow.com/questions/44518759/camunda-process-definition-deployment-via-api).

      Since deployments for resources that do not have a supported file extension (bpmn, bpmn20.xml, ...) do not work, wouldn't it be a good idea to avoid deployments with unsupported resource-names, or at least log a warning? It can be very nasty to analyse.

      I see these options:

      • check for suffix and raise exception
      • check for suffix and log warning
      • in case of addModelInstance: check for suffix and automatically extend the name based on the model type.

        Issue Links

          Activity

          Hide
          thorben.lindhauer Thorben Lindhauer added a comment -

          See https://app.camunda.com/jira/browse/CAM-7366. I believe raising exceptions or extending the resource name break backwards compatibility.

          Show
          thorben.lindhauer Thorben Lindhauer added a comment - See https://app.camunda.com/jira/browse/CAM-7366 . I believe raising exceptions or extending the resource name break backwards compatibility.
          Hide
          jan.galinski@holisticon.de Jan Galinski added a comment -

          I was not aware of that other issue ...

          But:

          I am with you: exception would break if some people would be using the deploymentBuilder in an unsupported way ... from which they would have no benefit as far as I can see, so I would not expect much resistance.

          But let's leave out exceptions then ... a warning would be a valid option, right?

          Plus: if for the modelinstances (only!) we would apply an .bpm/.dmn/.cmmn extension, why would that hurt? I mean you already are strict with the api usage and tell the engine what kind of deployment you want. I see no harm in that, the break of backward compatibility would only be that in a new release something works as expected that did not in a previous release ...

          Show
          jan.galinski@holisticon.de Jan Galinski added a comment - I was not aware of that other issue ... But: I am with you: exception would break if some people would be using the deploymentBuilder in an unsupported way ... from which they would have no benefit as far as I can see, so I would not expect much resistance. But let's leave out exceptions then ... a warning would be a valid option, right? Plus: if for the modelinstances (only!) we would apply an .bpm/.dmn/.cmmn extension, why would that hurt? I mean you already are strict with the api usage and tell the engine what kind of deployment you want. I see no harm in that, the break of backward compatibility would only be that in a new release something works as expected that did not in a previous release ...
          Hide
          thorben.lindhauer Thorben Lindhauer added a comment -

          With CAM-7366 a warning is already logged, however only for the addModelInstance APIs. For the other methods it does not make sense, because it is a valid use case to deploy non-BPMN/CMMN/DMN resources, e.g. a javascript file that the process invokes. A warning solely based on the file extension would be confusing.

          if for the modelinstances (only!) we would apply an .bpm/.dmn/.cmmn extension, why would that hurt?

          I get your point here and this is debatable. There's a chance that people deploy resources that they don't actually want to have interpreted (probably not realisitic but who knows). There's also a chance that people later change their deployment resource names by adding one of the suffixes and then may wonder that things like duplicate checking (which is based on deployment resource names) behave unexpectedly. In my personal opinion, this convenience feature does not outweigh the risks. Feel free to challenge this

          Show
          thorben.lindhauer Thorben Lindhauer added a comment - With CAM-7366 a warning is already logged, however only for the addModelInstance APIs. For the other methods it does not make sense, because it is a valid use case to deploy non-BPMN/CMMN/DMN resources, e.g. a javascript file that the process invokes. A warning solely based on the file extension would be confusing. if for the modelinstances (only!) we would apply an .bpm/.dmn/.cmmn extension, why would that hurt? I get your point here and this is debatable. There's a chance that people deploy resources that they don't actually want to have interpreted (probably not realisitic but who knows). There's also a chance that people later change their deployment resource names by adding one of the suffixes and then may wonder that things like duplicate checking (which is based on deployment resource names) behave unexpectedly. In my personal opinion, this convenience feature does not outweigh the risks. Feel free to challenge this
          Hide
          jan.galinski@holisticon.de Jan Galinski added a comment -

          Ok, I get your point.

          I get a warning logged when I try addModelInstance("foo", Bpmn.createExecutableContext().....done()) with 7.7.0?

          Must have missed that ... would be enough for me.

          Show
          jan.galinski@holisticon.de Jan Galinski added a comment - Ok, I get your point. I get a warning logged when I try addModelInstance("foo", Bpmn.createExecutableContext().....done()) with 7.7.0? Must have missed that ... would be enough for me.
          Hide
          thorben.lindhauer Thorben Lindhauer added a comment -

          Yup, that should work in 7.7.

          Show
          thorben.lindhauer Thorben Lindhauer added a comment - Yup, that should work in 7.7.
          Hide
          jan.galinski@holisticon.de Jan Galinski added a comment -

          then close this one .... thanks!

          Show
          jan.galinski@holisticon.de Jan Galinski added a comment - then close this one .... thanks!

            People

            • Assignee:
              Unassigned
              Reporter:
              jan.galinski@holisticon.de Jan Galinski
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: