Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix usage of session env variables and move some over to use env() #308

Merged
merged 2 commits into from
Jul 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions app/config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,12 @@ framework:
trusted_hosts: ~
session:
# https://symfony.com/doc/current/reference/configuration/framework.html#handler-id
# handler_id set to null will use default session handler from php.ini
handler_id: '%ezplatform.session.handler_id%'
save_path: '%ezplatform.session.save_path%'
# Note: eZ Publish also allows session name and session cookie configuration to be per SiteAccess, by
# if handler_id set to null will use default session handler from php.ini
handler_id: '%session.handler_id%'
save_path: '%session.save_path%'
# Note: eZ Platform also allows session name and session cookie configuration to be per SiteAccess, by
# default session name will be set to "eZSESSID{siteaccess_hash}" (unique session name per siteaccess)
# Further reading on sessions: http://doc.ezplatform.com/en/master/guide/sessions/
# Further reading on sessions: http://doc.ezplatform.com/en/latest/guide/sessions/
fragments: ~
http_method_override: true
assets: true
Expand Down
25 changes: 22 additions & 3 deletions app/config/default_parameters.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# This file contains defaults for optional parameters, which you can override in parameters.yml
parameters:
locale_fallback: en
ezplatform.session.handler_id: session.handler.native_file
ezplatform.session.save_path: '%kernel.project_dir%/var/sessions/%kernel.environment%'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this a BC break? I had ezplatform.session.save_path overridden on my project and this change broke my installation.

Copy link
Contributor Author

@andrerom andrerom Jul 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ezplatform.session.save_path was afaik introduced here and has not been made part of any release yet:
fc0e356

So it's maybe just because you use 2.2-dev? (we don't have BC policy for that) Or was it somewhere else before also?

However there is a BC break here on ezplatform.session.handler_id, which has been around across several branches for quite a while. So the renaming here should maybe anyway be renamed back to also avoid your issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you are right and I did it between those two changes. If this wasn't released yet I think we are ok with the ezplatform.session.save_path.

Copy link
Contributor Author

@andrerom andrerom Jul 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, but I changed them to:

  • session.handler_id
  • session.save_path

to be somewhat more consistent with other params.

However change on first param change is BC break as it's been around since 2.1.0 and 1.13.1.

So ok for you if I change it back to the following before 2.2.2?

  • ezplatform.session.handler_id
  • ezplatform.session.save_path

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, works for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8b3d4e0, looks ok to you? If so I'll merge up to the other repos

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok, thanks.


# A secret key that's used to generate certain security-related tokens
secret: '%env(SYMFONY_SECRET)%'
Expand Down Expand Up @@ -45,6 +43,14 @@ parameters:
# using EzSystemsPlatformHttpCacheBundle (default as of v1.12) which by design expires affected cache on changes
httpcache_default_ttl: '%env(HTTPCACHE_DEFAULT_TTL)%'

# Session save path as used by symfony session handlers (eg. used for dsn with redis)
session.save_path: '%env(SESSION_SAVE_PATH)%'

# Recommendation Bundle params
ez_recommendation.default.yoochoose.customer_id: '%env(RECOMMENDATIONS_CUSTOMER_ID)%'
ez_recommendation.default.yoochoose.license_key: '%env(RECOMMENDATIONS_LICENSE_KEY)%'
ez_recommendation.default.server_uri: '%env(PUBLIC_SERVER_URI)%'

# Fallback values for when environment variables do not exist

env(MAILER_HOST): 127.0.0.1
Expand All @@ -66,19 +72,32 @@ parameters:

env(HTTPCACHE_DEFAULT_TTL): 86400

env(SESSION_SAVE_PATH): '%kernel.project_dir%/var/sessions/%kernel.environment%'

env(RECOMMENDATIONS_CUSTOMER_ID): ~
env(RECOMMENDATIONS_LICENSE_KEY): ~
env(PUBLIC_SERVER_URI): ~


# Compile time handlers
## These are defined at compile time, and hence can't be set at runtime using env()
## app/config/env/generic.php takes care about letting you set them by env variables

## Log type is one of "stream", "error_log" or other types supported by monolog
# env: LOG_TYPE
log_type: stream

## Mail transport used by SwiftMailer
# env: MAILER_TRANSPORT
mailer_transport: smtp

# Purge type used by HttpCache system ("local", "varnish"/"http", and on ee also "fastly")
## Purge type used by HttpCache system ("local", "varnish"/"http", and on ee also "fastly")
# env: HTTPCACHE_PURGE_TYPE
purge_type: local

## Session handler, by default set to file based in order to be able to use %session.save_path%
# env: SESSION_HANDLER_ID
session.handler_id: session.handler.native_file

# Admin siteaccess group name
admin_group_name: admin_group
20 changes: 1 addition & 19 deletions app/config/env/generic.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,24 +68,6 @@
$container->setParameter('log_type', $value);
}

// EzSystemsRecommendationsBundle settings
// @todo Move to use env() and params
if ($value = getenv('RECOMMENDATIONS_CUSTOMER_ID')) {
$container->setParameter('ez_recommendation.default.yoochoose.customer_id', $value);
}

if ($value = getenv('RECOMMENDATIONS_LICENSE_KEY')) {
$container->setParameter('ez_recommendation.default.yoochoose.license_key', $value);
}

if ($value = getenv('PUBLIC_SERVER_URI')) {
$container->setParameter('ez_recommendation.default.server_uri', $value);
}

if ($value = getenv('SESSION_HANDLER_ID')) {
$container->setParameter('ezplatform.session.handler_id', $value);
}

if ($value = getenv('SESSION_SAVE_PATH')) {
$container->setParameter('ezplatform.session.save_path', $value);
$container->setParameter('session.handler_id', $value);
}
4 changes: 2 additions & 2 deletions app/config/env/platformsh.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@
continue;
}

$container->setParameter('ezplatform.session.handler_id', 'ezplatform.core.session.handler.native_redis');
$container->setParameter('session.handler_id', 'ezplatform.core.session.handler.native_redis');
$container->setParameter('session.save_path', sprintf('%s:%d', $endpoint['host'], $endpoint['port']));
}
} elseif (isset($relationships['rediscache'])) {
Expand All @@ -113,7 +113,7 @@
continue;
}

$container->setParameter('ezplatform.session.handler_id', 'ezplatform.core.session.handler.native_redis');
$container->setParameter('session.handler_id', 'ezplatform.core.session.handler.native_redis');
$container->setParameter('session.save_path', sprintf('%s:%d', $endpoint['host'], $endpoint['port']));
}
}
6 changes: 5 additions & 1 deletion app/config/parameters.yml.dist
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
# This file is a "template" of what your parameters.yml file should look like
#
# NB!: Some parameters are on purpose placed in default_parameters.yml to not
# prompt on all possible parameters during install with no info to go with it.
# (This will change once we move to use Symfony flex)
parameters:
# A secret key that's used to generate certain security-related tokens
env(SYMFONY_SECRET): ThisEzPlatformTokenIsNotSoSecretChangeIt
env(SYMFONY_SECRET): ThisEzPlatformTokenIsNotSoSecret_PleaseChangeIt

# Settings for database backend used by Doctrine DBAL
# In turn used for default storage engine & default search engine (if legacy is configured as search engine)
Expand Down